开云体育

27 червня 2024?р. 11:48:46 GMT+03:00, Caleb Connolly <caleb.connolly@...> написав(-ла):

On 27/06/2024 10:37, Simon Glass wrote:

Hi Marek,

On Thu, 27 Jun 2024 at 00:57, Marek Vasut <marex@...> 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@...>
---
Cc: Ben Wolsieffer <benwolsieffer@...>
Cc: Caleb Connolly <caleb.connolly@...>
Cc: Chris Morgan <macromorgan@...>
Cc: Dragan Simic <dsimic@...>
Cc: Eugen Hristev <eugen.hristev@...>
Cc: Francesco Dolcini <francesco.dolcini@...>
Cc: Heinrich Schuchardt <xypron.glpk@...>
Cc: Jaehoon Chung <jh80.chung@...>
Cc: Jagan Teki <jagan@...>
Cc: Jonas Karlman <jonas@...>
Cc: Kever Yang <kever.yang@...>
Cc: Kostya Porotchkin <kostap@...>
Cc: Matteo Lisi <matteo.lisi@...>
Cc: Mattijs Korpershoek <mkorpershoek@...>
Cc: Max Krummenacher <max.krummenacher@...>
Cc: Neil Armstrong <neil.armstrong@...>
Cc: Patrice Chotard <patrice.chotard@...>
Cc: Patrick Delaunay <patrick.delaunay@...>
Cc: Philipp Tomsich <philipp.tomsich@...>
Cc: Quentin Schulz <quentin.schulz@...>
Cc: Sam Day <me@...>
Cc: Simon Glass <sjg@...>
Cc: Sumit Garg <sumit.garg@...>
Cc: Svyatoslav Ryhel <clamor95@...>
Cc: Thierry Reding <treding@...>
Cc: Tom Rini <trini@...>
Cc: Volodymyr Babchuk <Volodymyr_Babchuk@...>
Cc: [email protected]
Cc: [email protected]
Cc: u-boot@...
Cc: u-boot@...
Cc: uboot-stm32@...
---
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.

Best regards,
Svyatoslav R.

Should we have a step in the init sequence where we set up the
regulators, by calling regulators_enable_boot_on() ?

Regards,
Simon

Hi Marek,

On Thu, 27 Jun 2024 at 17:05, Marek Vasut <marex@...> wrote:

On 6/27/24 10:37 AM, Simon Glass wrote:

Hi Marek,

Hi,

[...]

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

Actually, it is reading only the bare minimum very early in bind, the
always-on and boot-on, the rest is in pre_probe, i.e. later.

Yes, I see that. Also it is inevitable that these properties need to
be read before probe(), since they control whether to probe().

Also this seems to happen in SPL and again pre-reloc and again in
U-Boot post-reloc?

What does, the uclass post_bind ?

I mean that this code will be called in SPL (if the regulators are in
the DT there), U-Boot pre-reloc and post-reloc, each time turning on
the regulators. We need a way to control that, don't we?

Should we have a step in the init sequence where we set up the
regulators, by calling regulators_enable_boot_on() ?

Let's not do this , the entire point of this series is to get rid of
those functions and do the regulator configuration the same way LED
subsystem does it -- by probing always-on/boot-on regulators and
configuring them correctly on probe.

To me regulators_enable_boot_on() and the like was always an
inconsistently applied workaround for missing DM_FLAG_PROBE_AFTER_BIND ,
which is now implemented, so such workarounds can be removed.

That patch seemed to slip under the radar, sent and applied on the
same day! We really need to add a test for it, BTW.

My concern is that this might cause us ordering problems. We perhaps
want the regulators to be done first. If drivers are probed which use
regulators, then presumably they will enable them. Consider this:

- LED driver auto-probes
- probes I2C bus 2
- probes LDO1 which is autoset so turns on
- LDO1 probes, but is already running
- LDO2 probes, which is autoset so turns on

So long as it is OK to enable the regulators in any order, then this
seems fine. But is it? How do we handle the case where are particular
sequence is needed?

Regards,
Simon

чт, 27 черв. 2024?р. о 12:26 Simon Glass <sjg@...> пише:

Hi Svyatoslav,

On Thu, 27 Jun 2024 at 10:09, Svyatoslav <clamor95@...> wrote:

27 червня 2024?р. 11:48:46 GMT+03:00, Caleb Connolly <caleb.connolly@...> написав(-ла):

On 27/06/2024 10:37, Simon Glass wrote:

Hi Marek,

On Thu, 27 Jun 2024 at 00:57, Marek Vasut <marex@...> 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@...>
---
Cc: Ben Wolsieffer <benwolsieffer@...>
Cc: Caleb Connolly <caleb.connolly@...>
Cc: Chris Morgan <macromorgan@...>
Cc: Dragan Simic <dsimic@...>
Cc: Eugen Hristev <eugen.hristev@...>
Cc: Francesco Dolcini <francesco.dolcini@...>
Cc: Heinrich Schuchardt <xypron.glpk@...>
Cc: Jaehoon Chung <jh80.chung@...>
Cc: Jagan Teki <jagan@...>
Cc: Jonas Karlman <jonas@...>
Cc: Kever Yang <kever.yang@...>
Cc: Kostya Porotchkin <kostap@...>
Cc: Matteo Lisi <matteo.lisi@...>
Cc: Mattijs Korpershoek <mkorpershoek@...>
Cc: Max Krummenacher <max.krummenacher@...>
Cc: Neil Armstrong <neil.armstrong@...>
Cc: Patrice Chotard <patrice.chotard@...>
Cc: Patrick Delaunay <patrick.delaunay@...>
Cc: Philipp Tomsich <philipp.tomsich@...>
Cc: Quentin Schulz <quentin.schulz@...>
Cc: Sam Day <me@...>
Cc: Simon Glass <sjg@...>
Cc: Sumit Garg <sumit.garg@...>
Cc: Svyatoslav Ryhel <clamor95@...>
Cc: Thierry Reding <treding@...>
Cc: Tom Rini <trini@...>
Cc: Volodymyr Babchuk <Volodymyr_Babchuk@...>
Cc: [email protected]
Cc: [email protected]
Cc: u-boot@...
Cc: u-boot@...
Cc: uboot-stm32@...
---
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).

Best regards,
Svyatoslav R.

Should we have a step in the init sequence where we set up the
regulators, by calling regulators_enable_boot_on() ?

Regards,
Simon

Hi Caleb,

On Thu, 27 Jun 2024 at 09:48, Caleb Connolly <caleb.connolly@...> wrote:

On 27/06/2024 10:37, Simon Glass wrote:

Hi Marek,

On Thu, 27 Jun 2024 at 00:57, Marek Vasut <marex@...> 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@...>
---
Cc: Ben Wolsieffer <benwolsieffer@...>
Cc: Caleb Connolly <caleb.connolly@...>
Cc: Chris Morgan <macromorgan@...>
Cc: Dragan Simic <dsimic@...>
Cc: Eugen Hristev <eugen.hristev@...>
Cc: Francesco Dolcini <francesco.dolcini@...>
Cc: Heinrich Schuchardt <xypron.glpk@...>
Cc: Jaehoon Chung <jh80.chung@...>
Cc: Jagan Teki <jagan@...>
Cc: Jonas Karlman <jonas@...>
Cc: Kever Yang <kever.yang@...>
Cc: Kostya Porotchkin <kostap@...>
Cc: Matteo Lisi <matteo.lisi@...>
Cc: Mattijs Korpershoek <mkorpershoek@...>
Cc: Max Krummenacher <max.krummenacher@...>
Cc: Neil Armstrong <neil.armstrong@...>
Cc: Patrice Chotard <patrice.chotard@...>
Cc: Patrick Delaunay <patrick.delaunay@...>
Cc: Philipp Tomsich <philipp.tomsich@...>
Cc: Quentin Schulz <quentin.schulz@...>
Cc: Sam Day <me@...>
Cc: Simon Glass <sjg@...>
Cc: Sumit Garg <sumit.garg@...>
Cc: Svyatoslav Ryhel <clamor95@...>
Cc: Thierry Reding <treding@...>
Cc: Tom Rini <trini@...>
Cc: Volodymyr Babchuk <Volodymyr_Babchuk@...>
Cc: [email protected]
Cc: [email protected]
Cc: u-boot@...
Cc: u-boot@...
Cc: uboot-stm32@...
---
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.

That seems like a great idea to me, in general. The fact that SPL sets
up the MMU on armv8 makes it more practical.

But for this series I believe we are going to have to define what
happens in what phase. We have power_init_board() which is the old way
of doing this...but perhaps we could use that as a way to start up
regulators which are needed.

As to my question about whether this happens in SPL / pre-reloc /
proper, I forgot that we have the bootph tags for that, so it should
be fine. The main issue is that in U-Boot proper we will re-init the
regulators even though that has already been done. Probably that can
be handled by Kconfig or a flag in SPL handoff.

Also this seems to happen in SPL and again pre-reloc and again in
U-Boot post-reloc?

Should we have a step in the init sequence where we set up the
regulators, by calling regulators_enable_boot_on() ?

Regards,
Simon

On 27/06/2024 10:37, Simon Glass wrote:

Hi Marek,
On Thu, 27 Jun 2024 at 00:57, Marek Vasut <marex@...> 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@...>
---
Cc: Ben Wolsieffer <benwolsieffer@...>
Cc: Caleb Connolly <caleb.connolly@...>
Cc: Chris Morgan <macromorgan@...>
Cc: Dragan Simic <dsimic@...>
Cc: Eugen Hristev <eugen.hristev@...>
Cc: Francesco Dolcini <francesco.dolcini@...>
Cc: Heinrich Schuchardt <xypron.glpk@...>
Cc: Jaehoon Chung <jh80.chung@...>
Cc: Jagan Teki <jagan@...>
Cc: Jonas Karlman <jonas@...>
Cc: Kever Yang <kever.yang@...>
Cc: Kostya Porotchkin <kostap@...>
Cc: Matteo Lisi <matteo.lisi@...>
Cc: Mattijs Korpershoek <mkorpershoek@...>
Cc: Max Krummenacher <max.krummenacher@...>
Cc: Neil Armstrong <neil.armstrong@...>
Cc: Patrice Chotard <patrice.chotard@...>
Cc: Patrick Delaunay <patrick.delaunay@...>
Cc: Philipp Tomsich <philipp.tomsich@...>
Cc: Quentin Schulz <quentin.schulz@...>
Cc: Sam Day <me@...>
Cc: Simon Glass <sjg@...>
Cc: Sumit Garg <sumit.garg@...>
Cc: Svyatoslav Ryhel <clamor95@...>
Cc: Thierry Reding <treding@...>
Cc: Tom Rini <trini@...>
Cc: Volodymyr Babchuk <Volodymyr_Babchuk@...>
Cc: [email protected]
Cc: [email protected]
Cc: u-boot@...
Cc: u-boot@...
Cc: uboot-stm32@...
---
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?
Should we have a step in the init sequence where we set up the
regulators, by calling regulators_enable_boot_on() ?
Regards,
Simon

--
// Caleb (they/them)

Hi Svyatoslav,

On Thu, 27 Jun 2024 at 11:34, Svyatoslav Ryhel <clamor95@...> wrote:

чт, 27 черв. 2024?р. о 12:26 Simon Glass <sjg@...> пише:

Hi Svyatoslav,

On Thu, 27 Jun 2024 at 10:09, Svyatoslav <clamor95@...> wrote:

27 червня 2024?р. 11:48:46 GMT+03:00, Caleb Connolly <caleb.connolly@...> написав(-ла):

On 27/06/2024 10:37, Simon Glass wrote:

Hi Marek,

On Thu, 27 Jun 2024 at 00:57, Marek Vasut <marex@...> 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@...>
---
Cc: Ben Wolsieffer <benwolsieffer@...>
Cc: Caleb Connolly <caleb.connolly@...>
Cc: Chris Morgan <macromorgan@...>
Cc: Dragan Simic <dsimic@...>
Cc: Eugen Hristev <eugen.hristev@...>
Cc: Francesco Dolcini <francesco.dolcini@...>
Cc: Heinrich Schuchardt <xypron.glpk@...>
Cc: Jaehoon Chung <jh80.chung@...>
Cc: Jagan Teki <jagan@...>
Cc: Jonas Karlman <jonas@...>
Cc: Kever Yang <kever.yang@...>
Cc: Kostya Porotchkin <kostap@...>
Cc: Matteo Lisi <matteo.lisi@...>
Cc: Mattijs Korpershoek <mkorpershoek@...>
Cc: Max Krummenacher <max.krummenacher@...>
Cc: Neil Armstrong <neil.armstrong@...>
Cc: Patrice Chotard <patrice.chotard@...>
Cc: Patrick Delaunay <patrick.delaunay@...>
Cc: Philipp Tomsich <philipp.tomsich@...>
Cc: Quentin Schulz <quentin.schulz@...>
Cc: Sam Day <me@...>
Cc: Simon Glass <sjg@...>
Cc: Sumit Garg <sumit.garg@...>
Cc: Svyatoslav Ryhel <clamor95@...>
Cc: Thierry Reding <treding@...>
Cc: Tom Rini <trini@...>
Cc: Volodymyr Babchuk <Volodymyr_Babchuk@...>
Cc: [email protected]
Cc: [email protected]
Cc: u-boot@...
Cc: u-boot@...
Cc: uboot-stm32@...
---
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

On 6/27/24 10:37 AM, Simon Glass wrote:

Hi Marek,

Hi,

[...]

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

Actually, it is reading only the bare minimum very early in bind, the always-on and boot-on, the rest is in pre_probe, i.e. later.

Also this seems to happen in SPL and again pre-reloc and again in
U-Boot post-reloc?

What does, the uclass post_bind ?

Should we have a step in the init sequence where we set up the
regulators, by calling regulators_enable_boot_on() ?

Let's not do this , the entire point of this series is to get rid of those functions and do the regulator configuration the same way LED subsystem does it -- by probing always-on/boot-on regulators and configuring them correctly on probe.

To me regulators_enable_boot_on() and the like was always an inconsistently applied workaround for missing DM_FLAG_PROBE_AFTER_BIND , which is now implemented, so such workarounds can be removed.

On 27/06/2024 11:26, Simon Glass wrote:

Hi Caleb,
On Thu, 27 Jun 2024 at 09:48, Caleb Connolly <caleb.connolly@...> wrote:

On 27/06/2024 10:37, Simon Glass wrote:

Hi Marek,

On Thu, 27 Jun 2024 at 00:57, Marek Vasut <marex@...> 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@...>
---
Cc: Ben Wolsieffer <benwolsieffer@...>
Cc: Caleb Connolly <caleb.connolly@...>
Cc: Chris Morgan <macromorgan@...>
Cc: Dragan Simic <dsimic@...>
Cc: Eugen Hristev <eugen.hristev@...>
Cc: Francesco Dolcini <francesco.dolcini@...>
Cc: Heinrich Schuchardt <xypron.glpk@...>
Cc: Jaehoon Chung <jh80.chung@...>
Cc: Jagan Teki <jagan@...>
Cc: Jonas Karlman <jonas@...>
Cc: Kever Yang <kever.yang@...>
Cc: Kostya Porotchkin <kostap@...>
Cc: Matteo Lisi <matteo.lisi@...>
Cc: Mattijs Korpershoek <mkorpershoek@...>
Cc: Max Krummenacher <max.krummenacher@...>
Cc: Neil Armstrong <neil.armstrong@...>
Cc: Patrice Chotard <patrice.chotard@...>
Cc: Patrick Delaunay <patrick.delaunay@...>
Cc: Philipp Tomsich <philipp.tomsich@...>
Cc: Quentin Schulz <quentin.schulz@...>
Cc: Sam Day <me@...>
Cc: Simon Glass <sjg@...>
Cc: Sumit Garg <sumit.garg@...>
Cc: Svyatoslav Ryhel <clamor95@...>
Cc: Thierry Reding <treding@...>
Cc: Tom Rini <trini@...>
Cc: Volodymyr Babchuk <Volodymyr_Babchuk@...>
Cc: [email protected]
Cc: [email protected]
Cc: u-boot@...
Cc: u-boot@...
Cc: uboot-stm32@...
---
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.

That seems like a great idea to me, in general. The fact that SPL sets
up the MMU on armv8 makes it more practical.

Well, on qcom we don't use SPL (yet?), we did have a cyclical dependency since we rely on DTB for the memory layout, although I have some patches to do all the memory parsing in board_fdt_blob_setup() since that's needed for multi-dtb FIT. So I guess we could enable caches at the same time.

But for this series I believe we are going to have to define what
happens in what phase. We have power_init_board() which is the old way
of doing this...but perhaps we could use that as a way to start up
regulators which are needed.
As to my question about whether this happens in SPL / pre-reloc /
proper, I forgot that we have the bootph tags for that, so it should
be fine. The main issue is that in U-Boot proper we will re-init the
regulators even though that has already been done. Probably that can
be handled by Kconfig or a flag in SPL handoff.

Ensuring that it isn't done multiple times sounds like the right approach to me.

Also this seems to happen in SPL and again pre-reloc and again in
U-Boot post-reloc?

Should we have a step in the init sequence where we set up the
regulators, by calling regulators_enable_boot_on() ?

Regards,
Simon

--
// Caleb (they/them)

Hi Tom,

A set of changes to switch meson dwc3/usb PHY to set_mode callback for switching USB mode,
and a driver declaration fixup for meson_axg_gpio.

Thanks,
Neil

The following changes since commit a7eada24327a40f7ef6c55c220e119839c9d4227:

Merge tag 'v2024.07-rc5' into next (2024-06-24 13:34:52 -0600)

are available in the Git repository at:

tags/u-boot-amlogic-20240701

for you to fetch changes up to 96e1a156e90a0674b21c56e7121b17a9211176ec:

meson: Correct driver declaration for meson_axg_gpio (2024-06-26 11:19:57 +0200)

----------------------------------------------------------------
- Switch meson dwc3/usb PHY to set_mode callback for switching USB mode

----------------------------------------------------------------
Neil Armstrong (5):
phy: meson-gxl-usb2: add set_mode callback
usb: dwc3: meson-gxl: switch to generic_phy_set_mode
phy: meson-gxl-usb2: remove phy_meson_gxl_usb2_set_mode
usb: dwc3: meson-gxl: drop usb-gx.h and make dwc3_meson_gxl_force_mode static
usb: dwc3: meson-g12a: drop usb.h and make dwc3_meson_g12a_force_mode static

Simon Glass (1):
meson: Correct driver declaration for meson_axg_gpio

arch/arm/include/asm/arch-meson/usb-gx.h | 17 ---------------
arch/arm/include/asm/arch-meson/usb.h | 12 -----------
drivers/phy/meson-gxl-usb2.c | 30 ++++++++++++++++-----------
drivers/pinctrl/meson/pinctrl-meson-axg-pmx.c | 2 +-
drivers/pinctrl/meson/pinctrl-meson-axg.c | 4 ++--
drivers/pinctrl/meson/pinctrl-meson-axg.h | 2 +-
drivers/pinctrl/meson/pinctrl-meson-g12a.c | 4 ++--
drivers/usb/dwc3/dwc3-meson-g12a.c | 2 +-
drivers/usb/dwc3/dwc3-meson-gxl.c | 18 +++++++++-------
9 files changed, 35 insertions(+), 56 deletions(-)
delete mode 100644 arch/arm/include/asm/arch-meson/usb-gx.h
delete mode 100644 arch/arm/include/asm/arch-meson/usb.h

This function is never called, drop it.

Signed-off-by: Marek Vasut <marex@...>
---
Cc: Ben Wolsieffer <benwolsieffer@...>
Cc: Caleb Connolly <caleb.connolly@...>
Cc: Chris Morgan <macromorgan@...>
Cc: Dragan Simic <dsimic@...>
Cc: Eugen Hristev <eugen.hristev@...>
Cc: Francesco Dolcini <francesco.dolcini@...>
Cc: Heinrich Schuchardt <xypron.glpk@...>
Cc: Jaehoon Chung <jh80.chung@...>
Cc: Jagan Teki <jagan@...>
Cc: Jonas Karlman <jonas@...>
Cc: Kever Yang <kever.yang@...>
Cc: Kostya Porotchkin <kostap@...>
Cc: Matteo Lisi <matteo.lisi@...>
Cc: Mattijs Korpershoek <mkorpershoek@...>
Cc: Max Krummenacher <max.krummenacher@...>
Cc: Neil Armstrong <neil.armstrong@...>
Cc: Patrice Chotard <patrice.chotard@...>
Cc: Patrick Delaunay <patrick.delaunay@...>
Cc: Philipp Tomsich <philipp.tomsich@...>
Cc: Quentin Schulz <quentin.schulz@...>
Cc: Sam Day <me@...>
Cc: Simon Glass <sjg@...>
Cc: Sumit Garg <sumit.garg@...>
Cc: Svyatoslav Ryhel <clamor95@...>
Cc: Thierry Reding <treding@...>
Cc: Tom Rini <trini@...>
Cc: Volodymyr Babchuk <Volodymyr_Babchuk@...>
Cc: [email protected]
Cc: [email protected]
Cc: u-boot@...
Cc: u-boot@...
Cc: uboot-stm32@...
---
drivers/power/regulator/regulator-uclass.c | 11 -----------
include/power/regulator.h | 14 +-------------
2 files changed, 1 insertion(+), 24 deletions(-)

diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/r=
egulator/regulator-uclass.c
index d52f273042f..bcbcec1567e 100644
--- a/drivers/power/regulator/regulator-uclass.c
+++ b/drivers/power/regulator/regulator-uclass.c
@@ -339,17 +339,6 @@ out:
return ret;
}
=20
-int regulator_unset(struct udevice *dev)
-{
- struct dm_regulator_uclass_plat *uc_pdata;
-
- uc_pdata =3D dev_get_uclass_plat(dev);
- if (uc_pdata && uc_pdata->force_off)
- return regulator_set_enable(dev, false);
-
- return -EMEDIUMTYPE;
-}
-
static void regulator_show(struct udevice *dev, int ret)
{
struct dm_regulator_uclass_plat *uc_pdata;
diff --git a/include/power/regulator.h b/include/power/regulator.h
index bb07a814c79..5363483d02a 100644
--- a/include/power/regulator.h
+++ b/include/power/regulator.h
@@ -430,7 +430,7 @@ int regulators_enable_boot_on(bool verbose);
*
* This disables all regulators which are marked to be off at boot time.
*
- * This effectively calls regulator_unset() for every regulator.
+ * This effectively does nothing.
*/
int regulators_enable_boot_off(bool verbose);
=20
@@ -453,18 +453,6 @@ int regulators_enable_boot_off(bool verbose);
*/
int regulator_autoset(struct udevice *dev);
=20
-/**
- * regulator_unset: turn off a regulator
- *
- * The setup depends on constraints found in device's uclass's platform =
data
- * (struct dm_regulator_uclass_platdata):
- *
- * - Disable - will set - if 'force_off' is set to true,
- *
- * The function returns on the first-encountered error.
- */
-int regulator_unset(struct udevice *dev);
-
/**
* regulator_autoset_by_name: setup the regulator given by its uclass's
* platform data name field. The setup depends on constraints found in d=
evice's
--=20
2.43.0

Turn regulators_enable_boot_on() and regulators_enable_boot_off() into
empty functions. Implement matching functionality in regulator_post_probe=
()
instead. The regulator_post_probe() is called for all regulators after th=
ey
probe, and regulators that have regulator-always-on or regulator-boot-on =
DT
properties now always probe due to DM_FLAG_PROBE_AFTER_BIND being set on
such regulators in regulator_post_bind().

Finally, fold regulator_unset() functionality into regulator_autoset().

Signed-off-by: Marek Vasut <marex@...>
---
Cc: Ben Wolsieffer <benwolsieffer@...>
Cc: Caleb Connolly <caleb.connolly@...>
Cc: Chris Morgan <macromorgan@...>
Cc: Dragan Simic <dsimic@...>
Cc: Eugen Hristev <eugen.hristev@...>
Cc: Francesco Dolcini <francesco.dolcini@...>
Cc: Heinrich Schuchardt <xypron.glpk@...>
Cc: Jaehoon Chung <jh80.chung@...>
Cc: Jagan Teki <jagan@...>
Cc: Jonas Karlman <jonas@...>
Cc: Kever Yang <kever.yang@...>
Cc: Kostya Porotchkin <kostap@...>
Cc: Matteo Lisi <matteo.lisi@...>
Cc: Mattijs Korpershoek <mkorpershoek@...>
Cc: Max Krummenacher <max.krummenacher@...>
Cc: Neil Armstrong <neil.armstrong@...>
Cc: Patrice Chotard <patrice.chotard@...>
Cc: Patrick Delaunay <patrick.delaunay@...>
Cc: Philipp Tomsich <philipp.tomsich@...>
Cc: Quentin Schulz <quentin.schulz@...>
Cc: Sam Day <me@...>
Cc: Simon Glass <sjg@...>
Cc: Sumit Garg <sumit.garg@...>
Cc: Svyatoslav Ryhel <clamor95@...>
Cc: Thierry Reding <treding@...>
Cc: Tom Rini <trini@...>
Cc: Volodymyr Babchuk <Volodymyr_Babchuk@...>
Cc: [email protected]
Cc: [email protected]
Cc: u-boot@...
Cc: u-boot@...
Cc: uboot-stm32@...
---
drivers/power/regulator/regulator-uclass.c | 60 +++++++---------------
1 file changed, 19 insertions(+), 41 deletions(-)

diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/r=
egulator/regulator-uclass.c
index ccc4ef33d83..d52f273042f 100644
--- a/drivers/power/regulator/regulator-uclass.c
+++ b/drivers/power/regulator/regulator-uclass.c
@@ -308,6 +308,11 @@ int regulator_autoset(struct udevice *dev)
return ret;
}
=20
+ if (uc_pdata->force_off) {
+ ret =3D regulator_set_enable(dev, false);
+ goto out;
+ }
+
if (!uc_pdata->always_on && !uc_pdata->boot_on) {
ret =3D -EMEDIUMTYPE;
goto out;
@@ -512,56 +517,28 @@ static int regulator_pre_probe(struct udevice *dev)
return 0;
}
=20
-int regulators_enable_boot_on(bool verbose)
+static int regulator_post_probe(struct udevice *dev)
{
- struct udevice *dev;
- struct uclass *uc;
int ret;
=20
- ret =3D uclass_get(UCLASS_REGULATOR, &uc);
- if (ret)
+ ret =3D regulator_autoset(dev);
+ if (ret && ret !=3D -EMEDIUMTYPE && ret !=3D ENOSYS)
return ret;
- for (uclass_first_device(UCLASS_REGULATOR, &dev);
- dev;
- uclass_next_device(&dev)) {
- ret =3D regulator_autoset(dev);
- if (ret =3D=3D -EMEDIUMTYPE) {
- ret =3D 0;
- continue;
- }
- if (verbose)
- regulator_show(dev, ret);
- if (ret =3D=3D -ENOSYS)
- ret =3D 0;
- }
=20
- return ret;
+ if (_DEBUG)
+ regulator_show(dev, ret);
+
+ return 0;
}
=20
-int regulators_enable_boot_off(bool verbose)
+int regulators_enable_boot_on(bool verbose)
{
- struct udevice *dev;
- struct uclass *uc;
- int ret;
-
- ret =3D uclass_get(UCLASS_REGULATOR, &uc);
- if (ret)
- return ret;
- for (uclass_first_device(UCLASS_REGULATOR, &dev);
- dev;
- uclass_next_device(&dev)) {
- ret =3D regulator_unset(dev);
- if (ret =3D=3D -EMEDIUMTYPE) {
- ret =3D 0;
- continue;
- }
- if (verbose)
- regulator_show(dev, ret);
- if (ret =3D=3D -ENOSYS)
- ret =3D 0;
- }
+ return 0;
+}
=20
- return ret;
+int regulators_enable_boot_off(bool verbose)
+{
+ return 0;
}
=20
UCLASS_DRIVER(regulator) =3D {
@@ -569,5 +546,6 @@ UCLASS_DRIVER(regulator) =3D {
.name =3D "regulator",
.post_bind =3D regulator_post_bind,
.pre_probe =3D regulator_pre_probe,
+ .post_probe =3D regulator_post_probe,
.per_device_plat_auto =3D sizeof(struct dm_regulator_uclass_plat),
};
--=20
2.43.0

In case a regulator DT node contains regulator-always-on or regulator-boo=
t-on
property, make sure the regulator gets correctly configured by U-Boot on =
start
up. Unconditionally probe such regulator drivers. This is a preparatory p=
atch
for introduction of .regulator_post_probe() which would trigger the regul=
ator
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@...>
---
Cc: Ben Wolsieffer <benwolsieffer@...>
Cc: Caleb Connolly <caleb.connolly@...>
Cc: Chris Morgan <macromorgan@...>
Cc: Dragan Simic <dsimic@...>
Cc: Eugen Hristev <eugen.hristev@...>
Cc: Francesco Dolcini <francesco.dolcini@...>
Cc: Heinrich Schuchardt <xypron.glpk@...>
Cc: Jaehoon Chung <jh80.chung@...>
Cc: Jagan Teki <jagan@...>
Cc: Jonas Karlman <jonas@...>
Cc: Kever Yang <kever.yang@...>
Cc: Kostya Porotchkin <kostap@...>
Cc: Matteo Lisi <matteo.lisi@...>
Cc: Mattijs Korpershoek <mkorpershoek@...>
Cc: Max Krummenacher <max.krummenacher@...>
Cc: Neil Armstrong <neil.armstrong@...>
Cc: Patrice Chotard <patrice.chotard@...>
Cc: Patrick Delaunay <patrick.delaunay@...>
Cc: Philipp Tomsich <philipp.tomsich@...>
Cc: Quentin Schulz <quentin.schulz@...>
Cc: Sam Day <me@...>
Cc: Simon Glass <sjg@...>
Cc: Sumit Garg <sumit.garg@...>
Cc: Svyatoslav Ryhel <clamor95@...>
Cc: Thierry Reding <treding@...>
Cc: Tom Rini <trini@...>
Cc: Volodymyr Babchuk <Volodymyr_Babchuk@...>
Cc: [email protected]
Cc: [email protected]
Cc: u-boot@...
Cc: u-boot@...
Cc: uboot-stm32@...
---
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/r=
egulator/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 =3D "regulator-name";
=20
uc_pdata =3D dev_get_uclass_plat(dev);
+ uc_pdata->always_on =3D dev_read_bool(dev, "regulator-always-on");
+ uc_pdata->boot_on =3D dev_read_bool(dev, "regulator-boot-on");
=20
/* Regulator's mandatory constraint */
uc_pdata->name =3D dev_read_string(dev, property);
@@ -444,13 +446,21 @@ static int regulator_post_bind(struct udevice *dev)
return -EINVAL;
}
=20
- 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;
+ }
=20
- 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);
=20
- return -EINVAL;
+ return 0;
}
=20
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 =3D dev_read_u32_default(dev, "regulator-max-microamp"=
,
-ENODATA);
- uc_pdata->always_on =3D dev_read_bool(dev, "regulator-always-on");
- uc_pdata->boot_on =3D dev_read_bool(dev, "regulator-boot-on");
uc_pdata->ramp_delay =3D dev_read_u32_default(dev, "regulator-ramp-dela=
y",
0);
uc_pdata->force_off =3D dev_read_bool(dev, "regulator-force-boot-off");
--=20
2.43.0

Both regulators_enable_boot_on/off() are unused and superseded by
regulator uclass regulator_post_probe(). Remove both functions.

Signed-off-by: Marek Vasut <marex@...>
---
Cc: Ben Wolsieffer <benwolsieffer@...>
Cc: Caleb Connolly <caleb.connolly@...>
Cc: Chris Morgan <macromorgan@...>
Cc: Dragan Simic <dsimic@...>
Cc: Eugen Hristev <eugen.hristev@...>
Cc: Francesco Dolcini <francesco.dolcini@...>
Cc: Heinrich Schuchardt <xypron.glpk@...>
Cc: Jaehoon Chung <jh80.chung@...>
Cc: Jagan Teki <jagan@...>
Cc: Jonas Karlman <jonas@...>
Cc: Kever Yang <kever.yang@...>
Cc: Kostya Porotchkin <kostap@...>
Cc: Matteo Lisi <matteo.lisi@...>
Cc: Mattijs Korpershoek <mkorpershoek@...>
Cc: Max Krummenacher <max.krummenacher@...>
Cc: Neil Armstrong <neil.armstrong@...>
Cc: Patrice Chotard <patrice.chotard@...>
Cc: Patrick Delaunay <patrick.delaunay@...>
Cc: Philipp Tomsich <philipp.tomsich@...>
Cc: Quentin Schulz <quentin.schulz@...>
Cc: Sam Day <me@...>
Cc: Simon Glass <sjg@...>
Cc: Sumit Garg <sumit.garg@...>
Cc: Svyatoslav Ryhel <clamor95@...>
Cc: Thierry Reding <treding@...>
Cc: Tom Rini <trini@...>
Cc: Volodymyr Babchuk <Volodymyr_Babchuk@...>
Cc: [email protected]
Cc: [email protected]
Cc: u-boot@...
Cc: u-boot@...
Cc: uboot-stm32@...
---
arch/arm/mach-rockchip/board.c | 8 --------
arch/arm/mach-snapdragon/board.c | 1 -
arch/arm/mach-tegra/board2.c | 3 ---
board/Marvell/octeontx2_cn913x/board.c | 5 -----
.../amlogic/odroid-go-ultra/odroid-go-ultra.c | 2 --
board/dhelectronics/dh_stm32mp1/board.c | 2 --
board/engicam/stm32mp1/stm32mp1.c | 3 ---
board/google/veyron/veyron.c | 6 ------
board/samsung/common/exynos5-dt.c | 4 ----
board/st/stm32mp1/stm32mp1.c | 2 --
drivers/power/regulator/regulator-uclass.c | 10 ----------
include/power/regulator.h | 20 -------------------
12 files changed, 66 deletions(-)

diff --git a/arch/arm/mach-rockchip/board.c b/arch/arm/mach-rockchip/boar=
d.c
index 8a57b8217ff..a1933300653 100644
--- a/arch/arm/mach-rockchip/board.c
+++ b/arch/arm/mach-rockchip/board.c
@@ -202,14 +202,6 @@ int board_late_init(void)
=20
int board_init(void)
{
- int ret;
-
-#ifdef CONFIG_DM_REGULATOR
- ret =3D regulators_enable_boot_on(false);
- if (ret)
- debug("%s: Cannot enable boot on regulator\n", __func__);
-#endif
-
return 0;
}
=20
diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/=
board.c
index b439a19ec7e..feb2a1157de 100644
--- a/arch/arm/mach-snapdragon/board.c
+++ b/arch/arm/mach-snapdragon/board.c
@@ -162,7 +162,6 @@ void __weak qcom_board_init(void)
=20
int board_init(void)
{
- regulators_enable_boot_on(false);
show_psci_version();
qcom_of_fixup_nodes();
qcom_board_init();
diff --git a/arch/arm/mach-tegra/board2.c b/arch/arm/mach-tegra/board2.c
index 479137e457c..193220f29c9 100644
--- a/arch/arm/mach-tegra/board2.c
+++ b/arch/arm/mach-tegra/board2.c
@@ -187,9 +187,6 @@ int board_init(void)
warmboot_prepare_code(TEGRA_LP0_ADDR, TEGRA_LP0_SIZE);
#endif
=20
- /* Set up boot-on regulators */
- regulators_enable_boot_on(_DEBUG);
-
return nvidia_board_init();
}
=20
diff --git a/board/Marvell/octeontx2_cn913x/board.c b/board/Marvell/octeo=
ntx2_cn913x/board.c
index 3d20cfb2fab..3ffe15d42b8 100644
--- a/board/Marvell/octeontx2_cn913x/board.c
+++ b/board/Marvell/octeontx2_cn913x/board.c
@@ -23,11 +23,6 @@ int board_early_init_f(void)
=20
int board_early_init_r(void)
{
- if (CONFIG_IS_ENABLED(DM_REGULATOR)) {
- /* Check if any existing regulator should be turned down */
- regulators_enable_boot_off(false);
- }
-
return 0;
}
=20
diff --git a/board/amlogic/odroid-go-ultra/odroid-go-ultra.c b/board/amlo=
gic/odroid-go-ultra/odroid-go-ultra.c
index 8f3f2045d74..f9412071737 100644
--- a/board/amlogic/odroid-go-ultra/odroid-go-ultra.c
+++ b/board/amlogic/odroid-go-ultra/odroid-go-ultra.c
@@ -16,7 +16,5 @@ int mmc_get_env_dev(void)
=20
int board_init(void)
{
- regulators_enable_boot_on(_DEBUG);
-
return 0;
}
diff --git a/board/dhelectronics/dh_stm32mp1/board.c b/board/dhelectronic=
s/dh_stm32mp1/board.c
index 4f4f537fee5..24c5f37c12f 100644
--- a/board/dhelectronics/dh_stm32mp1/board.c
+++ b/board/dhelectronics/dh_stm32mp1/board.c
@@ -622,8 +622,6 @@ static void board_init_regulator_av96(void)
static void board_init_regulator(void)
{
board_init_regulator_av96();
-
- regulators_enable_boot_on(_DEBUG);
}
#else
static inline int board_get_regulator_buck3_nvm_uv_av96(int *uv)
diff --git a/board/engicam/stm32mp1/stm32mp1.c b/board/engicam/stm32mp1/s=
tm32mp1.c
index bc2af66d8e9..56557d56429 100644
--- a/board/engicam/stm32mp1/stm32mp1.c
+++ b/board/engicam/stm32mp1/stm32mp1.c
@@ -37,9 +37,6 @@ int checkboard(void)
/* board dependent setup after realloc */
int board_init(void)
{
- if (IS_ENABLED(CONFIG_DM_REGULATOR))
- regulators_enable_boot_on(_DEBUG);
-
return 0;
}
=20
diff --git a/board/google/veyron/veyron.c b/board/google/veyron/veyron.c
index 53c3435c92f..055c1c5fc6d 100644
--- a/board/google/veyron/veyron.c
+++ b/board/google/veyron/veyron.c
@@ -61,12 +61,6 @@ static int veyron_init(void)
if (ret)
return ret;
=20
- ret =3D regulators_enable_boot_on(false);
- if (ret) {
- debug("%s: Cannot enable boot on regulators\n", __func__);
- return ret;
- }
-
return 0;
}
#endif
diff --git a/board/samsung/common/exynos5-dt.c b/board/samsung/common/exy=
nos5-dt.c
index 56862bcb34d..68edd1ec282 100644
--- a/board/samsung/common/exynos5-dt.c
+++ b/board/samsung/common/exynos5-dt.c
@@ -88,10 +88,6 @@ int exynos_power_init(void)
if (ret =3D=3D -ENODEV)
return 0;
=20
- ret =3D regulators_enable_boot_on(false);
- if (ret)
- return ret;
-
ret =3D exynos_set_regulator("vdd_mif", 1100000);
if (ret)
return ret;
diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
index 97532a8156f..d5e5e776d2a 100644
--- a/board/st/stm32mp1/stm32mp1.c
+++ b/board/st/stm32mp1/stm32mp1.c
@@ -665,8 +665,6 @@ int board_init(void)
if (board_is_stm32mp15x_dk2())
board_stm32mp15x_dk2_init();
=20
- regulators_enable_boot_on(_DEBUG);
-
/*
* sysconf initialisation done only when U-Boot is running in secure
* done in TF-A for TFABOOT.
diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/r=
egulator/regulator-uclass.c
index bcbcec1567e..7d59d0f0cbd 100644
--- a/drivers/power/regulator/regulator-uclass.c
+++ b/drivers/power/regulator/regulator-uclass.c
@@ -520,16 +520,6 @@ static int regulator_post_probe(struct udevice *dev)
return 0;
}
=20
-int regulators_enable_boot_on(bool verbose)
-{
- return 0;
-}
-
-int regulators_enable_boot_off(bool verbose)
-{
- return 0;
-}
-
UCLASS_DRIVER(regulator) =3D {
.id =3D UCLASS_REGULATOR,
.name =3D "regulator",
diff --git a/include/power/regulator.h b/include/power/regulator.h
index 5363483d02a..f49bcf76b68 100644
--- a/include/power/regulator.h
+++ b/include/power/regulator.h
@@ -414,26 +414,6 @@ int regulator_get_mode(struct udevice *dev);
*/
int regulator_set_mode(struct udevice *dev, int mode_id);
=20
-/**
- * regulators_enable_boot_on() - enable regulators needed for boot
- *
- * This enables all regulators which are marked to be on at boot time. T=
his
- * only works for regulators which don't have a range for voltage/curren=
t,
- * since in that case it is not possible to know which value to use.
- *
- * This effectively calls regulator_autoset() for every regulator.
- */
-int regulators_enable_boot_on(bool verbose);
-
-/**
- * regulators_enable_boot_off() - disable regulators needed for boot
- *
- * This disables all regulators which are marked to be off at boot time.
- *
- * This effectively does nothing.
- */
-int regulators_enable_boot_off(bool verbose);
-
/**
* regulator_autoset: setup the voltage/current on a regulator
*
--=20
2.43.0

Hi,

On Sun, 23 Jun 2024 14:30:19 -0600, Simon Glass wrote:

This series includes a number of mostly unrelated changes which are in
service of running U-Boot on a lab using Labgrid.

Changes in v2:
- Add new patch to update u-boot.cfg with CFG_... options

Simon Glass (14):
trace: Update test to tolerate different trace-cmd version
dm: core: Enhance comments on bind_drivers_pass()
initcall: Correct use of relocation offset
am33xx: Provide a function to set up the debug UART
sunxi: Mark scp as optional
google: Disable TPMv2 on most Chromebooks
meson: Correct driver declaration for meson_axg_gpio
test: Make bootstd init run only on sandbox
log: Allow tests to pass with CONFIG_LOGF_FUNC_PAD set
test: dm: Show failing driver name
test: Decode exceptions only with sandbox
test: Check help output
Update u-boot.cfg to include CFG also
smbios: Correct error handling when writing tables

[...]

Thanks, Applied to (u-boot-amlogic-next)

[07/14] meson: Correct driver declaration for meson_axg_gpio


--
Neil

Hi,

On Thu, 20 Jun 2024 09:42:49 +0200, Neil Armstrong wrote:

Switch to set_mode callback now it's available and in the same
time make public functions static and drop useless mach-meson
headers that are no more needed.

Thanks, Applied to (u-boot-amlogic-next)

[1/5] phy: meson-gxl-usb2: add set_mode callback

[2/5] usb: dwc3: meson-gxl: switch to generic_phy_set_mode

[3/5] phy: meson-gxl-usb2: remove phy_meson_gxl_usb2_set_mode

[4/5] usb: dwc3: meson-gxl: drop usb-gx.h and make dwc3_meson_gxl_force_mode static

[5/5] usb: dwc3: meson-g12a: drop usb.h and make dwc3_meson_g12a_force_mode static


--
Neil

Hi,

On Tue, 11 Jun 2024 14:01:14 -0600, Simon Glass wrote:

Labgrid provides access to a hardware lab in an automated way. It is
possible to boot U-Boot on boards in the lab without physically touching
them. It relies on relays, USB UARTs and SD muxes, among other things.

By way of background, about 4 years ago I wrong a thing called Labman[1]
which allowed my lab of about 30 devices to be operated remotely, using
tbot for the console and build integration. While it worked OK and I
used it for many bisects, I didn't take it any further.

[...]

Thanks, Applied to (u-boot-amlogic-next)

[18/42] meson: Correct driver declaration for meson_axg_gpio


--
Neil

On 6/24/24 11:11 AM, Neil Armstrong wrote:

Hi Marek,

Hi,

On 20/06/2024 09:42, Neil Armstrong wrote:

Switch to set_mode callback now it's available and in the same
time make public functions static and drop useless mach-meson
headers that are no more needed.

Thanks for the review, is it ok to apply it via my tree ?

Yes please

Hello Arseniy,

Am Sun, Jun 02, 2024 at 11:08:34PM +0300 schrieb Arseniy Krasnov:

Based on Linux kernel:
commit f922bd798bb9 ("mtd: rawnand: add an option to specify NAND chip as a boot device")

Allow to define a NAND chip as a boot device. This can be helpful
for the selection of the ECC algorithm and strength in case the boot
ROM supports only a subset of controller provided options.

Signed-off-by: Arseniy Krasnov <avkrasnov@...>
---
drivers/mtd/nand/raw/nand_base.c | 3 +++
include/linux/mtd/rawnand.h | 6 ++++++
2 files changed, 9 insertions(+)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index c40a0f23d7..ed605b4af5 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -4458,6 +4458,9 @@ static int nand_dt_init(struct mtd_info *mtd, struct nand_chip *chip, ofnode nod
if (ret == 16)
chip->options |= NAND_BUSWIDTH_16;

+ if (ofnode_read_bool(node, "nand-is-boot-medium"))
+ chip->options |= NAND_IS_BOOT_MEDIUM;
+
if (ofnode_read_bool(node, "nand-on-flash-bbt"))
chip->bbt_options |= NAND_BBT_USE_FLASH;

diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index fb002ae641..4eb880d8fb 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -218,6 +218,12 @@ enum nand_ecc_algo {
/* Device needs 3rd row address cycle */
#define NAND_ROW_ADDR_3 0x00004000

+/*
+ * Whether the NAND chip is a boot medium. Drivers might use this information
+ * to select ECC algorithms supported by the boot ROM or similar restrictions.
+ */
+#define NAND_IS_BOOT_MEDIUM 0x00400000
+
/* Options valid for Samsung large page devices */
#define NAND_SAMSUNG_LP_OPTIONS NAND_CACHEPRG

The entries in that file are ordered numerically. Comparing with the
same file in Linux this should go before NAND_KEEP_TIMINGS, not
anywhere in between. Makes later diffs and ports from Linux easier.

Greets
Alex

--
2.35.0

Hi, thanks, ok! What build problem ? With this [PATCH v1] mtd: rawnand: macronix: OTP access for MX30LFxG18AC ?

Thanks

Hi

Yes I have seen and I will review today and cross-check. Did you lunch
the testing on the other patches series? I would like to merge
at all but we were having some build breakage

Michael

On Mon, Jun 24, 2024 at 7:16?AM Arseniy Krasnov
<avkrasnov@...> wrote:

Hi, sorry, pls ping :)

Thanks

On 02.06.2024 23:08, Arseniy Krasnov wrote:

Based on Linux kernel:
commit f922bd798bb9 ("mtd: rawnand: add an option to specify NAND chip as a boot device")

Allow to define a NAND chip as a boot device. This can be helpful
for the selection of the ECC algorithm and strength in case the boot
ROM supports only a subset of controller provided options.

Signed-off-by: Arseniy Krasnov <avkrasnov@...>
---
drivers/mtd/nand/raw/nand_base.c | 3 +++
include/linux/mtd/rawnand.h | 6 ++++++
2 files changed, 9 insertions(+)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index c40a0f23d7..ed605b4af5 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -4458,6 +4458,9 @@ static int nand_dt_init(struct mtd_info *mtd, struct nand_chip *chip, ofnode nod
if (ret == 16)
chip->options |= NAND_BUSWIDTH_16;

+ if (ofnode_read_bool(node, "nand-is-boot-medium"))
+ chip->options |= NAND_IS_BOOT_MEDIUM;
+
if (ofnode_read_bool(node, "nand-on-flash-bbt"))
chip->bbt_options |= NAND_BBT_USE_FLASH;

diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index fb002ae641..4eb880d8fb 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -218,6 +218,12 @@ enum nand_ecc_algo {
/* Device needs 3rd row address cycle */
#define NAND_ROW_ADDR_3 0x00004000

+/*
+ * Whether the NAND chip is a boot medium. Drivers might use this information
+ * to select ECC algorithms supported by the boot ROM or similar restrictions.
+ */
+#define NAND_IS_BOOT_MEDIUM 0x00400000
+
/* Options valid for Samsung large page devices */
#define NAND_SAMSUNG_LP_OPTIONS NAND_CACHEPRG

--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael@...
__________________________________

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info@...
www.amarulasolutions.com