* [PATCH 1/4] gpio: add a way to parse a GPIO now and request it later
2026-08-28 10:52 [PATCH 0/4] dm: gpio: read GPIOs in of_to_plat(), request them in probe() Mehmet Fide
@ 2026-08-28 10:52 ` Mehmet Fide
2026-09-10 14:06 ` Simon Glass
2026-08-28 10:52 ` [PATCH 2/4] regulator: claim the enable GPIO at probe time, not in of_to_plat() Mehmet Fide
` (3 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Mehmet Fide @ 2026-08-28 10:52 UTC (permalink / raw)
To: Simon Glass
Cc: Tom Rini, Jaehoon Chung, Peng Fan, Vincent Jardin, Ye Li,
Michal Simek, Aristo Chen, u-boot, Mehmet Fide
From: Mehmet Fide <mehmet.fide@screeningeagle.com>
A driver's of_to_plat() method must only read the devicetree; probing
other devices or claiming resources belongs in probe(). Several drivers
nevertheless call gpio_request_by_name() from of_to_plat(), because it
is the only way to pick a GPIO out of the devicetree: it resolves and
probes the controller and claims the GPIO in one go. On boards where
the consumer's pinctrl touches the same pad, the pinctrl state, applied
between the two phases, then undoes the direction the early claim set.
Split the two halves: gpio_parse_by_name() reads the phandle into a new
struct gpio_dt_desc without touching any device, and can be stored in
the platform data; gpio_request_parsed() resolves the controller,
claims the GPIO and applies the direction flags, for use in probe().
Signed-off-by: Mehmet Fide <mehmet.fide@screeningeagle.com>
---
drivers/gpio/gpio-uclass.c | 33 ++++++++++++++++++++++++
include/asm-generic/gpio.h | 51 ++++++++++++++++++++++++++++++++++++++
test/dm/gpio.c | 29 ++++++++++++++++++++++
3 files changed, 113 insertions(+)
diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
index 4d40738e5aa..b65500eafb0 100644
--- a/drivers/gpio/gpio-uclass.c
+++ b/drivers/gpio/gpio-uclass.c
@@ -1216,6 +1216,39 @@ int gpio_request_by_name_nodev(ofnode node, const char *list_name, int index,
index > 0);
}
+int gpio_parse_by_name(struct udevice *dev, const char *list_name, int index,
+ int flags, struct gpio_dt_desc *dt)
+{
+ int ret;
+
+ dt->present = false;
+ ret = dev_read_phandle_with_args(dev, list_name, "#gpio-cells", 0,
+ index, &dt->args);
+ if (ret)
+ return ret;
+ dt->list_name = list_name;
+ dt->flags = flags;
+ dt->present = true;
+
+ return 0;
+}
+
+int gpio_request_parsed(struct udevice *dev, const struct gpio_dt_desc *dt,
+ struct gpio_desc *desc)
+{
+ struct ofnode_phandle_args args;
+
+ if (!dt->present) {
+ gpio_desc_init(desc, NULL, 0);
+ return -ENOENT;
+ }
+
+ args = dt->args;
+ return gpio_request_tail(0, ofnode_get_name(dev_ofnode(dev)), &args,
+ dt->list_name, 0, desc, dt->flags, false,
+ NULL);
+}
+
int gpio_request_by_name(struct udevice *dev, const char *list_name, int index,
struct gpio_desc *desc, int flags)
{
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index a21c606f2b8..9d64f78e642 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -574,6 +574,57 @@ int gpio_claim_vector(const int *gpio_num_array, const char *fmt);
int gpio_request_by_name(struct udevice *dev, const char *list_name,
int index, struct gpio_desc *desc, int flags);
+/**
+ * struct gpio_dt_desc - devicetree description of a GPIO, not yet requested
+ *
+ * Filled by gpio_parse_by_name() from an of_to_plat() method, which must not
+ * probe other devices or claim the GPIO, and consumed by
+ * gpio_request_parsed() from the probe() method.
+ *
+ * @args: phandle arguments naming the controller node and the GPIO
+ * @list_name: name of the devicetree property that was parsed
+ * @flags: GPIOD_... flags requested by the caller
+ * @present: true if the property exists and was parsed
+ */
+struct gpio_dt_desc {
+ struct ofnode_phandle_args args;
+ const char *list_name;
+ int flags;
+ bool present;
+};
+
+/**
+ * gpio_parse_by_name() - read a GPIO from the devicetree without requesting it
+ *
+ * This only reads the devicetree, so it is safe to call from an of_to_plat()
+ * method; the GPIO controller is neither probed nor touched. Request the GPIO
+ * in the probe() method with gpio_request_parsed().
+ *
+ * @dev: Device requesting the GPIO
+ * @list_name: Name of devicetree property containing the GPIO
+ * @index: Index of the GPIO in the list of GPIOs
+ * @flags: GPIOD_... flags to use when the GPIO is requested later
+ * @dt: Returns the parsed description
+ * Return: 0 if OK, -ENOENT if the property is missing, other -ve on error
+ */
+int gpio_parse_by_name(struct udevice *dev, const char *list_name, int index,
+ int flags, struct gpio_dt_desc *dt);
+
+/**
+ * gpio_request_parsed() - request a GPIO parsed by gpio_parse_by_name()
+ *
+ * This does the second half of gpio_request_by_name(): resolve the
+ * controller, claim the GPIO and apply the direction flags. Call it from the
+ * probe() method.
+ *
+ * @dev: Device requesting the GPIO (used for the request label)
+ * @dt: Description returned by gpio_parse_by_name()
+ * @desc: Returns the GPIO description, ready for use
+ * Return: 0 if OK, -ENOENT if @dt holds no GPIO, other -ve on error
+ */
+int gpio_request_parsed(struct udevice *dev, const struct gpio_dt_desc *dt,
+ struct gpio_desc *desc);
+
/* gpio_request_by_line_name - Locate and request a GPIO by line name
*
* Request a GPIO using the offset of the provided line name in the
diff --git a/test/dm/gpio.c b/test/dm/gpio.c
index 0fb05b5ca06..4df9cb902eb 100644
--- a/test/dm/gpio.c
+++ b/test/dm/gpio.c
@@ -257,6 +257,35 @@ static int dm_test_gpio_opendrain_opensource(struct unit_test_state *uts)
DM_TEST(dm_test_gpio_opendrain_opensource,
UTF_SCAN_PDATA | UTF_SCAN_FDT);
+/* Test parsing a GPIO in one phase and requesting it in another */
+static int dm_test_gpio_parse_request(struct unit_test_state *uts)
+{
+ struct gpio_dt_desc dt;
+ struct gpio_desc desc;
+ struct udevice *dev;
+
+ ut_assertok(uclass_get_device(UCLASS_TEST_FDT, 0, &dev));
+ ut_asserteq_str("a-test", dev->name);
+
+ /* parsing alone must not claim the GPIO */
+ ut_assertok(gpio_parse_by_name(dev, "test2-gpios", 1, GPIOD_IS_OUT,
+ &dt));
+ ut_asserteq(true, dt.present);
+
+ ut_assertok(gpio_request_parsed(dev, &dt, &desc));
+ ut_asserteq(GPIOF_OUTPUT, gpio_get_function(desc.dev, desc.offset,
+ NULL));
+ ut_assertok(dm_gpio_free(dev, &desc));
+
+ /* a missing property parses and requests as -ENOENT */
+ ut_asserteq(-ENOENT,
+ gpio_parse_by_name(dev, "no-such-gpios", 0, 0, &dt));
+ ut_asserteq(-ENOENT, gpio_request_parsed(dev, &dt, &desc));
+
+ return 0;
+}
+DM_TEST(dm_test_gpio_parse_request, UTF_SCAN_PDATA | UTF_SCAN_FDT);
+
/* Test that sandbox anonymous GPIOs work correctly */
static int dm_test_gpio_anon(struct unit_test_state *uts)
{
base-commit: 527115ef6783cec49e5610c523c124b399011361
--
2.54.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 1/4] gpio: add a way to parse a GPIO now and request it later
2026-08-28 10:52 ` [PATCH 1/4] gpio: add a way to parse a GPIO now and request it later Mehmet Fide
@ 2026-09-10 14:06 ` Simon Glass
2026-09-10 14:58 ` Mehmet Fide
0 siblings, 1 reply; 16+ messages in thread
From: Simon Glass @ 2026-09-10 14:06 UTC (permalink / raw)
To: mehmet.fide
Cc: Simon Glass, Tom Rini, Jaehoon Chung, Peng Fan, Vincent Jardin,
Ye Li, Michal Simek, Aristo Chen, u-boot, Mehmet Fide
Hi Mehmet,
On 2026-08-28T10:52:07, Mehmet Fide <mehmet.fide@gmail.com> wrote:
> gpio: add a way to parse a GPIO now and request it later
>
> A driver's of_to_plat() method must only read the devicetree; probing
> other devices or claiming resources belongs in probe(). Several drivers
> nevertheless call gpio_request_by_name() from of_to_plat(), because it
> is the only way to pick a GPIO out of the devicetree: it resolves and
> probes the controller and claims the GPIO in one go. On boards where
> the consumer's pinctrl touches the same pad, the pinctrl state, applied
> between the two phases, then undoes the direction the early claim set.
I like this approach. BTW clocks and reset lines have the same problem
so we could deal with those late as needed.
>
> Split the two halves: gpio_parse_by_name() reads the phandle into a new
> struct gpio_dt_desc without touching any device, and can be stored in
> the platform data; gpio_request_parsed() resolves the controller,
> claims the GPIO and applies the direction flags, for use in probe().
>
> Signed-off-by: Mehmet Fide <mehmet.fide@screeningeagle.com>
>
> drivers/gpio/gpio-uclass.c | 33 ++++++++++++++++++++++++++++++
> include/asm-generic/gpio.h | 51 ++++++++++++++++++++++++++++++++++++++++++++++
> test/dm/gpio.c | 29 ++++++++++++++++++++++++++
> 3 files changed, 113 insertions(+)
> diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
> @@ -1219,6 +1219,39 @@ int gpio_request_by_name_nodev(ofnode node, const char *list_name, int index,
> +int gpio_request_parsed(struct udevice *dev, const struct gpio_dt_desc *dt,
> + struct gpio_desc *desc)
> +{
> + struct ofnode_phandle_args args;
> +
> + if (!dt->present) {
> + gpio_desc_init(desc, NULL, 0);
> + return -ENOENT;
> + }
> +
> + args = dt->args;
> + return gpio_request_tail(0, ofnode_get_name(dev_ofnode(dev)), &args,
> + dt->list_name, 0, desc, dt->flags, false,
> + NULL);
> +}
The original index is dropped: gpio_request_by_name() passes 'index >
0' to gpio_request_tail() so the request label gains an index suffix
when index != 0 - see the '%s.%s%d' branch in gpio_request_tail().
gpio_request_parsed() hardcodes index 0 and add_index=false, so a
caller that parsed at index 3 ends up with the label 'nodename.gpios'
instead of 'nodename.gpios3'. Please store the index in gpio_dt_desc
and pass it through so the two entry points produce identical labels.
Your own test uses index 1 for 'test2-gpios' and quietly exercises
this - worth an ut_asserteq_str on the request label to lock it down.
> diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
> @@ -574,6 +574,57 @@ int gpio_claim_vector(const int *gpio_num_array, const char *fmt);
> +struct gpio_dt_desc {
> + struct ofnode_phandle_args args;
> + const char *list_name;
> + int flags;
> + bool present;
> +};
Note that list_name is a pointer with an implicit 'must outlive the
desc' contract - current callers pass string literals. Can you please
mention this in the kernel-doc so people don't (later) put a
stack-allocated buffer in that field.
Also gpio_dt_desc reads as a peer of gpio_desc when it is really a
pending/parsed version of one. gpio_desc_parsed, or gpio_dt_spec,
would make the two-phase relationship clearer. What do you think?
> diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
> @@ -574,6 +574,57 @@ int gpio_claim_vector(const int *gpio_num_array, const char *fmt);
> + * @dev: Device requesting the GPIO (used for the request label)
> + * @dt: Description returned by gpio_parse_by_name()
> + * @desc: Returns the GPIO description, ready for use
> + * Return: 0 if OK, -ENOENT if @dt holds no GPIO, other -ve on error
> + */
> +int gpio_request_parsed(struct udevice *dev, const struct gpio_dt_desc *dt,
> + struct gpio_desc *desc);
Please note in the kernel-doc that the -ENOENT return for a
not-present @dt is deliberate, so a caller with an optional GPIO can
skip the 'if (dt->present)' check and just tolerate -ENOENT - right
now regulator_common_probe() checks 'present' first and never sees
that path, which makes the -ENOENT branch look like dead code.
> diff --git a/test/dm/gpio.c b/test/dm/gpio.c
> @@ -257,6 +257,35 @@ static int dm_test_gpio_opendrain_opensource(struct unit_test_state *uts)
> + /* parsing alone must not claim the GPIO */
> + ut_assertok(gpio_parse_by_name(dev, "test2-gpios", 1, GPIOD_IS_OUT,
> + &dt));
> + ut_asserteq(true, dt.present);
The comment says 'parsing alone must not claim the GPIO' but the test
never verifies that - it just checks dt.present and then goes on to
request. Please add a gpio_get_function() check between the parse and
the request, mirroring what patch 4 does for the regulator. That is
the invariant this whole series is defending, and it belongs in the
gpio-level test too.
Regards,
Simon
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 1/4] gpio: add a way to parse a GPIO now and request it later
2026-09-10 14:06 ` Simon Glass
@ 2026-09-10 14:58 ` Mehmet Fide
0 siblings, 0 replies; 16+ messages in thread
From: Mehmet Fide @ 2026-09-10 14:58 UTC (permalink / raw)
To: Simon Glass
Cc: Tom Rini, Jaehoon Chung, Peng Fan, Vincent Jardin, Ye Li,
Michal Simek, Aristo Chen, u-boot, Mehmet Fide
Hi Simon,
On 2026-09-10 Simon Glass wrote:
> I like this approach. BTW clocks and reset lines have the same problem
> so we could deal with those late as needed.
Yes, the same two-phase shape fits them; I kept this series to GPIOs to
get the pattern agreed first.
> The original index is dropped: gpio_request_by_name() passes 'index >
> 0' to gpio_request_tail() so the request label gains an index suffix
> when index != 0
You are right. The three callers converted in patch 2 all use index 0,
so I never stored it, and the test happened to work because it only
checks the function, not the label. v2 stores the index in the struct
and passes it to gpio_request_tail() with add_index = index > 0, so
both entry points produce the same label, and the test asserts the
label through gpio_get_function()'s name pointer.
> Note that list_name is a pointer with an implicit 'must outlive the
> desc' contract
Will document. All current callers pass string literals.
> Also gpio_dt_desc reads as a peer of gpio_desc when it is really a
> pending/parsed version of one. gpio_desc_parsed, or gpio_dt_spec,
> would make the two-phase relationship clearer. What do you think?
gpio_dt_spec, then: it says what the thing is (a devicetree
specification of a GPIO) rather than what it is not, and Zephyr uses
the same name for the same idea, a GPIO described by the devicetree
and configured later.
> Please note in the kernel-doc that the -ENOENT return for a
> not-present @dt is deliberate, so a caller with an optional GPIO can
> skip the 'if (dt->present)' check and just tolerate -ENOENT
Will do, and patch 2 will use that contract instead of guarding on
present in two places (see the reply there).
> Please add a gpio_get_function() check between the parse and
> the request, mirroring what patch 4 does for the regulator.
Yes. test2-gpios index 1 is a4, so the test will assert GPIOF_UNUSED on
a4 after the parse and GPIOF_OUTPUT with the expected label after the
request.
Regards,
Mehmet
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/4] regulator: claim the enable GPIO at probe time, not in of_to_plat()
2026-08-28 10:52 [PATCH 0/4] dm: gpio: read GPIOs in of_to_plat(), request them in probe() Mehmet Fide
2026-08-28 10:52 ` [PATCH 1/4] gpio: add a way to parse a GPIO now and request it later Mehmet Fide
@ 2026-08-28 10:52 ` Mehmet Fide
2026-09-10 14:08 ` Simon Glass
2026-08-28 10:52 ` [PATCH 3/4] doc: driver-model: state that of_to_plat() must not probe or claim Mehmet Fide
` (2 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Mehmet Fide @ 2026-08-28 10:52 UTC (permalink / raw)
To: Simon Glass
Cc: Tom Rini, Jaehoon Chung, Peng Fan, Vincent Jardin, Ye Li,
Michal Simek, Aristo Chen, u-boot, Mehmet Fide
From: Mehmet Fide <mehmet.fide@screeningeagle.com>
regulator_common_of_to_plat() requests the enable GPIO, which probes
the GPIO controller and claims the pin while the consumer's platform
data is still being read. of_to_plat() must not do either: it runs
before the device's pinctrl state is applied, so on SoCs where the
direction lives in the pad register - the output enable on Vybrid is
one - the pinctrl undoes the direction the early claim just set, and a
fixed regulator configured as always-on powers nothing.
Parse the GPIO into the platform data in of_to_plat() and request it in
the new regulator_common_probe(), called from the fixed and fixed-clock
regulator probe methods. The gpio regulator's voltage-control GPIO has
the same problem and moves the same way.
Signed-off-by: Mehmet Fide <mehmet.fide@screeningeagle.com>
---
drivers/power/regulator/fixed.c | 11 ++++++++++
drivers/power/regulator/gpio-regulator.c | 21 +++++++++++++++---
drivers/power/regulator/regulator_common.c | 25 ++++++++++++++++++----
drivers/power/regulator/regulator_common.h | 3 +++
4 files changed, 53 insertions(+), 7 deletions(-)
diff --git a/drivers/power/regulator/fixed.c b/drivers/power/regulator/fixed.c
index 1dd137f493e..b8e3af0f0ca 100644
--- a/drivers/power/regulator/fixed.c
+++ b/drivers/power/regulator/fixed.c
@@ -38,6 +38,11 @@ static int fixed_regulator_of_to_plat(struct udevice *dev)
return regulator_common_of_to_plat(dev, plat, gpios ? "gpios" : "gpio");
}
+static int fixed_regulator_probe(struct udevice *dev)
+{
+ return regulator_common_probe(dev, dev_get_plat(dev));
+}
+
static int fixed_regulator_get_value(struct udevice *dev)
{
struct dm_regulator_uclass_plat *uc_pdata;
@@ -115,6 +120,11 @@ static int fixed_clock_regulator_set_enable(struct udevice *dev, bool enable)
static int fixed_clock_regulator_probe(struct udevice *dev)
{
struct fixed_clock_regulator_priv *priv = dev_get_priv(dev);
+ int ret;
+
+ ret = regulator_common_probe(dev, dev_get_plat(dev));
+ if (ret)
+ return ret;
priv->enable_clock = devm_clk_get(dev, NULL);
if (IS_ERR(priv->enable_clock))
@@ -150,6 +160,7 @@ U_BOOT_DRIVER(regulator_fixed) = {
.id = UCLASS_REGULATOR,
.ops = &fixed_regulator_ops,
.of_match = fixed_regulator_ids,
+ .probe = fixed_regulator_probe,
.of_to_plat = fixed_regulator_of_to_plat,
.plat_auto = sizeof(struct regulator_common_plat),
};
diff --git a/drivers/power/regulator/gpio-regulator.c b/drivers/power/regulator/gpio-regulator.c
index 787f8170234..703a96ff095 100644
--- a/drivers/power/regulator/gpio-regulator.c
+++ b/drivers/power/regulator/gpio-regulator.c
@@ -19,6 +19,7 @@
struct gpio_regulator_plat {
struct regulator_common_plat common;
+ struct gpio_dt_desc gpio_dt; /* parsed voltage GPIO, requested in probe */
struct gpio_desc gpio; /* GPIO for regulator voltage control */
int states[GPIO_REGULATOR_MAX_STATES];
int voltages[GPIO_REGULATOR_MAX_STATES];
@@ -28,7 +29,6 @@ static int gpio_regulator_of_to_plat(struct udevice *dev)
{
struct dm_regulator_uclass_plat *uc_pdata;
struct gpio_regulator_plat *plat;
- struct gpio_desc *gpio;
int ret, count, i, j;
u32 states_array[GPIO_REGULATOR_MAX_STATES * 2];
@@ -47,8 +47,8 @@ static int gpio_regulator_of_to_plat(struct udevice *dev)
* per gpio-regulator. As of now no instance with multiple
* gpios is presnt
*/
- gpio = &plat->gpio;
- ret = gpio_request_by_name(dev, "gpios", 0, gpio, GPIOD_IS_OUT);
+ ret = gpio_parse_by_name(dev, "gpios", 0, GPIOD_IS_OUT,
+ &plat->gpio_dt);
if (ret)
debug("regulator gpio - not found! Error: %d", ret);
@@ -76,6 +76,20 @@ static int gpio_regulator_of_to_plat(struct udevice *dev)
return regulator_common_of_to_plat(dev, &plat->common, "enable-gpios");
}
+static int gpio_regulator_probe(struct udevice *dev)
+{
+ struct gpio_regulator_plat *plat = dev_get_plat(dev);
+ int ret;
+
+ if (plat->gpio_dt.present) {
+ ret = gpio_request_parsed(dev, &plat->gpio_dt, &plat->gpio);
+ if (ret)
+ return ret;
+ }
+
+ return regulator_common_probe(dev, &plat->common);
+}
+
static int gpio_regulator_get_value(struct udevice *dev)
{
struct dm_regulator_uclass_plat *uc_pdata;
@@ -153,6 +167,7 @@ U_BOOT_DRIVER(gpio_regulator) = {
.id = UCLASS_REGULATOR,
.ops = &gpio_regulator_ops,
.of_match = gpio_regulator_ids,
+ .probe = gpio_regulator_probe,
.of_to_plat = gpio_regulator_of_to_plat,
.plat_auto = sizeof(struct gpio_regulator_plat),
};
diff --git a/drivers/power/regulator/regulator_common.c b/drivers/power/regulator/regulator_common.c
index c0387eff4fc..99a3cde436e 100644
--- a/drivers/power/regulator/regulator_common.c
+++ b/drivers/power/regulator/regulator_common.c
@@ -16,7 +16,6 @@ int regulator_common_of_to_plat(struct udevice *dev,
struct regulator_common_plat *plat,
const char *enable_gpio_name)
{
- struct gpio_desc *gpio;
int flags = GPIOD_IS_OUT;
int ret;
@@ -25,10 +24,10 @@ int regulator_common_of_to_plat(struct udevice *dev,
if (dev_read_bool(dev, "regulator-boot-on"))
flags |= GPIOD_IS_OUT_ACTIVE;
- /* Get optional enable GPIO desc */
- gpio = &plat->gpio;
+ /* Read the optional enable GPIO; it is requested in probe() */
if (CONFIG_IS_ENABLED(DM_GPIO)) {
- ret = gpio_request_by_name(dev, enable_gpio_name, 0, gpio, flags);
+ ret = gpio_parse_by_name(dev, enable_gpio_name, 0, flags,
+ &plat->gpio_dt);
if (ret) {
debug("Regulator '%s' optional enable GPIO - not found! Error: %d\n",
dev->name, ret);
@@ -49,6 +48,24 @@ int regulator_common_of_to_plat(struct udevice *dev,
return 0;
}
+int regulator_common_probe(struct udevice *dev,
+ struct regulator_common_plat *plat)
+{
+ int ret;
+
+ if (!CONFIG_IS_ENABLED(DM_GPIO) || !plat->gpio_dt.present)
+ return 0;
+
+ ret = gpio_request_parsed(dev, &plat->gpio_dt, &plat->gpio);
+ if (ret) {
+ debug("Regulator '%s' enable GPIO request failed: %d\n",
+ dev->name, ret);
+ return ret;
+ }
+
+ return 0;
+}
+
int regulator_common_get_enable(const struct udevice *dev,
struct regulator_common_plat *plat)
{
diff --git a/drivers/power/regulator/regulator_common.h b/drivers/power/regulator/regulator_common.h
index d4962899d83..43e32ac48f6 100644
--- a/drivers/power/regulator/regulator_common.h
+++ b/drivers/power/regulator/regulator_common.h
@@ -10,6 +10,7 @@
#include <asm/gpio.h>
struct regulator_common_plat {
+ struct gpio_dt_desc gpio_dt; /* parsed enable GPIO, requested in probe */
struct gpio_desc gpio; /* GPIO for regulator enable control */
unsigned int startup_delay_us;
unsigned int off_on_delay_us;
@@ -19,6 +20,8 @@ struct regulator_common_plat {
int regulator_common_of_to_plat(struct udevice *dev,
struct regulator_common_plat *plat, const
char *enable_gpio_name);
+int regulator_common_probe(struct udevice *dev,
+ struct regulator_common_plat *plat);
int regulator_common_get_enable(const struct udevice *dev,
struct regulator_common_plat *plat);
/*
--
2.54.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 2/4] regulator: claim the enable GPIO at probe time, not in of_to_plat()
2026-08-28 10:52 ` [PATCH 2/4] regulator: claim the enable GPIO at probe time, not in of_to_plat() Mehmet Fide
@ 2026-09-10 14:08 ` Simon Glass
2026-09-10 14:58 ` Mehmet Fide
0 siblings, 1 reply; 16+ messages in thread
From: Simon Glass @ 2026-09-10 14:08 UTC (permalink / raw)
To: mehmet.fide
Cc: Simon Glass, Tom Rini, Jaehoon Chung, Peng Fan, Vincent Jardin,
Ye Li, Michal Simek, Aristo Chen, u-boot, Mehmet Fide
Hi Mehmet,
On 2026-08-28T10:52:07, Mehmet Fide <mehmet.fide@gmail.com> wrote:
> regulator: claim the enable GPIO at probe time, not in of_to_plat()
>
> regulator_common_of_to_plat() requests the enable GPIO, which probes
> the GPIO controller and claims the pin while the consumer's platform
> data is still being read. of_to_plat() must not do either: it runs
> before the device's pinctrl state is applied, so on SoCs where the
> direction lives in the pad register - the output enable on Vybrid is
> one - the pinctrl undoes the direction the early claim just set, and a
> fixed regulator configured as always-on powers nothing.
>
> Parse the GPIO into the platform data in of_to_plat() and request it in
> the new regulator_common_probe(), called from the fixed and fixed-clock
> regulator probe methods. The gpio regulator's voltage-control GPIO has
> the same problem and moves the same way.
>
> Signed-off-by: Mehmet Fide <mehmet.fide@screeningeagle.com>
>
> drivers/power/regulator/fixed.c | 11 +++++++++++
> drivers/power/regulator/gpio-regulator.c | 21 ++++++++++++++++++---
> drivers/power/regulator/regulator_common.c | 25 +++++++++++++++++++++----
> drivers/power/regulator/regulator_common.h | 3 +++
> 4 files changed, 53 insertions(+), 7 deletions(-)
> diff --git a/drivers/power/regulator/gpio-regulator.c b/drivers/power/regulator/gpio-regulator.c
> @@ -28,7 +29,6 @@ static int gpio_regulator_of_to_plat(struct udevice *dev)
> - gpio = &plat->gpio;
> - ret = gpio_request_by_name(dev, "gpios", 0, gpio, GPIOD_IS_OUT);
> + ret = gpio_parse_by_name(dev, "gpios", 0, GPIOD_IS_OUT,
> + &plat->gpio_dt);
> if (ret)
> debug("regulator gpio - not found! Error: %d", ret);
The debug message is stale once the request moves to probe() - a parse
failure no longer means the GPIO controller could not give us the pin,
only that the phandle could not be read. Please reword (something like
'gpio-regulator: cannot parse voltage GPIO') and add the missing '\n'.
> diff --git a/drivers/power/regulator/gpio-regulator.c b/drivers/power/regulator/gpio-regulator.c
> @@ -76,6 +76,20 @@ static int gpio_regulator_of_to_plat(struct udevice *dev)
> +static int gpio_regulator_probe(struct udevice *dev)
> +{
> + struct gpio_regulator_plat *plat = dev_get_plat(dev);
> + int ret;
> +
> + if (plat->gpio_dt.present) {
> + ret = gpio_request_parsed(dev, &plat->gpio_dt, &plat->gpio);
> + if (ret)
> + return ret;
> + }
> +
> + return regulator_common_probe(dev, &plat->common);
> +}
This quietly changes behaviour: the old of_to_plat() only
debug()-logged a failure from gpio_request_by_name() and carried on,
whereas probe() now aborts the device for any non-ENOENT error. That
may be right, but please call it out in the commit message so anyone
bisecting a regulator that used to 'work' knows where to look.
Also the 'if (plat->gpio_dt.present)' guard duplicates the check that
gpio_request_parsed() already does - the whole point of the -ENOENT
return in patch 1 is that the caller does not need to guard.
regulator_common_probe() below takes the other approach and reads
better; please pick one and use it in both places.
> diff --git a/drivers/power/regulator/fixed.c b/drivers/power/regulator/fixed.c
> @@ -115,6 +120,11 @@ static int fixed_clock_regulator_set_enable(struct udevice *dev, bool enable)
> static int fixed_clock_regulator_probe(struct udevice *dev)
> {
> struct fixed_clock_regulator_priv *priv = dev_get_priv(dev);
> + int ret;
> +
> + ret = regulator_common_probe(dev, dev_get_plat(dev));
> + if (ret)
> + return ret;
Does fixed-clock ever actually carry an enable GPIO in the wild? It
shares of_to_plat() with the plain fixed regulator so the parse
happens, but set_enable() only pokes the clock, so the GPIO would be
claimed and never used. If the answer is 'no', I would rather not add
the call here at all; if 'sometimes', a line in the commit message
explaining why fixed-clock needs it too would help.
> diff --git a/drivers/power/regulator/regulator_common.h b/drivers/power/regulator/regulator_common.h
> @@ -10,6 +10,7 @@
> struct regulator_common_plat {
> + struct gpio_dt_desc gpio_dt; /* parsed enable GPIO, requested in probe */
> struct gpio_desc gpio; /* GPIO for regulator enable control */
Once the device is probed, gpio_dt is dead weight in plat - the
phandle args, list_name pointer and flags are all baked into gpio. Not
critical, but on boards with a lot of regulators this doubles the
per-device plat footprint for no runtime benefit. Worth a note in the
commit message that this is a deliberate trade for keeping the
two-phase split simple.
Regards,
Simon
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 2/4] regulator: claim the enable GPIO at probe time, not in of_to_plat()
2026-09-10 14:08 ` Simon Glass
@ 2026-09-10 14:58 ` Mehmet Fide
0 siblings, 0 replies; 16+ messages in thread
From: Mehmet Fide @ 2026-09-10 14:58 UTC (permalink / raw)
To: Simon Glass
Cc: Tom Rini, Jaehoon Chung, Peng Fan, Vincent Jardin, Ye Li,
Michal Simek, Aristo Chen, u-boot, Mehmet Fide
Hi Simon,
On 2026-09-10 Simon Glass wrote:
> The debug message is stale once the request moves to probe()
Will reword and add the missing newline.
> This quietly changes behaviour: the old of_to_plat() only
> debug()-logged a failure from gpio_request_by_name() and carried on,
> whereas probe() now aborts the device for any non-ENOENT error.
For the fixed regulator there is no change: regulator_common_of_to_plat()
already returned any error other than -ENOENT after the debug(), so a
missing controller or a busy pin failed the device before this series
too, only at a different stage. The change is real for the
gpio-regulator's voltage GPIO, which used to debug() and carry on even
though the driver cannot do anything without that GPIO. I will state
that in the commit message.
> Also the 'if (plat->gpio_dt.present)' guard duplicates the check that
> gpio_request_parsed() already does
Agreed, v2 drops the guards and tolerates -ENOENT in both places.
> Does fixed-clock ever actually carry an enable GPIO in the wild?
Not in this tree: the only regulator-fixed-clock node is the Colibri
iMX6ULL Ethernet PHY supply and it has no GPIO. The binding allows one
and Linux drives it next to the clock, but our set_enable() ignores it.
I added the request only to keep what the shared of_to_plat() did
before. Since nothing observable depends on it, v2 drops the call from
fixed-clock's probe() and says so in the commit message. Teaching
set_enable() to drive the GPIO as Linux does would be a separate patch,
if anyone needs it.
> Once the device is probed, gpio_dt is dead weight in plat
Yes. priv is not allocated when of_to_plat() runs, so plat is the only
place the parsed description can live; I will note the trade-off in the
commit message.
Regards,
Mehmet
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/4] doc: driver-model: state that of_to_plat() must not probe or claim
2026-08-28 10:52 [PATCH 0/4] dm: gpio: read GPIOs in of_to_plat(), request them in probe() Mehmet Fide
2026-08-28 10:52 ` [PATCH 1/4] gpio: add a way to parse a GPIO now and request it later Mehmet Fide
2026-08-28 10:52 ` [PATCH 2/4] regulator: claim the enable GPIO at probe time, not in of_to_plat() Mehmet Fide
@ 2026-08-28 10:52 ` Mehmet Fide
2026-09-10 14:09 ` Simon Glass
2026-08-28 10:52 ` [PATCH 4/4] test: dm: check the fixed regulator claims its GPIO at probe time Mehmet Fide
2026-09-10 14:10 ` [0/4] dm: gpio: read GPIOs in of_to_plat(), request them in probe() Simon Glass
4 siblings, 1 reply; 16+ messages in thread
From: Mehmet Fide @ 2026-08-28 10:52 UTC (permalink / raw)
To: Simon Glass
Cc: Tom Rini, Jaehoon Chung, Peng Fan, Vincent Jardin, Ye Li,
Michal Simek, Aristo Chen, u-boot, Mehmet Fide
From: Mehmet Fide <mehmet.fide@screeningeagle.com>
The design document says decoding the devicetree belongs in
of_to_plat(), but not what the method must not do. Spell out the rule
that has always been implied by the phase separation: no probing of
other devices and no claiming of resources, and point at the
parse-now-request-later GPIO helpers as the pattern to follow.
Signed-off-by: Mehmet Fide <mehmet.fide@screeningeagle.com>
---
doc/develop/driver-model/design.rst | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/doc/develop/driver-model/design.rst b/doc/develop/driver-model/design.rst
index 633545944d1..7f60c75f4d5 100644
--- a/doc/develop/driver-model/design.rst
+++ b/doc/develop/driver-model/design.rst
@@ -759,6 +759,15 @@ The steps are:
6. The device is marked 'plat valid'.
+The of_to_plat() method must only read the devicetree. It must not probe
+other devices or claim resources such as GPIOs or clocks: ofdata is read
+before the device's pinctrl state is applied, so a pin claimed here can have
+its configuration undone a moment later, and probing another device from this
+method defeats the lazy-probing model. When a resource is named in the
+devicetree, read its description into the platform data here - for a GPIO,
+gpio_parse_by_name() - and claim it in probe(), for a GPIO with
+gpio_request_parsed().
+
Note that ofdata reading is always done (for a child and all its parents)
before probing starts. Thus devices go through two distinct states when
probing: reading platform data and actually touching the hardware to bring
--
2.54.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 3/4] doc: driver-model: state that of_to_plat() must not probe or claim
2026-08-28 10:52 ` [PATCH 3/4] doc: driver-model: state that of_to_plat() must not probe or claim Mehmet Fide
@ 2026-09-10 14:09 ` Simon Glass
2026-09-10 14:58 ` Mehmet Fide
0 siblings, 1 reply; 16+ messages in thread
From: Simon Glass @ 2026-09-10 14:09 UTC (permalink / raw)
To: mehmet.fide
Cc: Simon Glass, Tom Rini, Jaehoon Chung, Peng Fan, Vincent Jardin,
Ye Li, Michal Simek, Aristo Chen, u-boot, Mehmet Fide
Hi Mehmet,
On 2026-08-28T10:52:07, Mehmet Fide <mehmet.fide@gmail.com> wrote:
> doc: driver-model: state that of_to_plat() must not probe or claim
>
> The design document says decoding the devicetree belongs in
> of_to_plat(), but not what the method must not do. Spell out the rule
> that has always been implied by the phase separation: no probing of
> other devices and no claiming of resources, and point at the
> parse-now-request-later GPIO helpers as the pattern to follow.
>
> Signed-off-by: Mehmet Fide <mehmet.fide@screeningeagle.com>
>
> doc/develop/driver-model/design.rst | 9 +++++++++
> 1 file changed, 9 insertions(+)
> +devicetree, read its description into the platform data here - for a GPIO,
> +gpio_parse_by_name() - and claim it in probe(), for a GPIO with
> +gpio_request_parsed().
The final sentence reads oddly - 'for a GPIO' appears twice and the
second clause parses as 'claim it in probe(), for a GPIO with
gpio_request_parsed()'. Something like 'read its description into the
platform data here with gpio_parse_by_name() and claim it in probe()
with gpio_request_parsed()' would be clearer, and you could then
generalise: the same 'parse now, request later' shape applies to
clocks, resets and phys too, even if the helpers do not exist yet.
> diff --git a/doc/develop/driver-model/design.rst b/doc/develop/driver-model/design.rst
> @@ -759,6 +759,15 @@ The steps are:
> +The of_to_plat() method must only read the devicetree. It must not probe
> +other devices or claim resources such as GPIOs or clocks: ofdata is read
> +before the device's pinctrl state is applied, so a pin claimed here can have
> +its configuration undone a moment later, and probing another device from this
> +method defeats the lazy-probing model. When a resource is named in the
> +devicetree, read its description into the platform data here - for a GPIO,
> +gpio_parse_by_name() - and claim it in probe(), for a GPIO with
> +gpio_request_parsed().
The new paragraph is good, but a few lines down (around line 789) the
existing text says the opposite:
It is important that the of_to_plat() method does not actually probe
the device itself. However there are cases where other devices must be
probed in the of_to_plat() method. An example is where a device
requires a GPIO for it to operate. To select a GPIO obviously requires
that the GPIO device is probed. This is OK when used by common, core
devices such as GPIO, clock, interrupts, reset and the like.
That is exactly the pattern this series is outlawing. Please delete or
rewrite that paragraph in the same patch so the document does not
contradict itself.
Regards,
Simon
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 3/4] doc: driver-model: state that of_to_plat() must not probe or claim
2026-09-10 14:09 ` Simon Glass
@ 2026-09-10 14:58 ` Mehmet Fide
0 siblings, 0 replies; 16+ messages in thread
From: Mehmet Fide @ 2026-09-10 14:58 UTC (permalink / raw)
To: Simon Glass
Cc: Tom Rini, Jaehoon Chung, Peng Fan, Vincent Jardin, Ye Li,
Michal Simek, Aristo Chen, u-boot, Mehmet Fide
Hi Simon,
On 2026-09-10 Simon Glass wrote:
> The final sentence reads oddly - 'for a GPIO' appears twice
It does; I was trying to keep the rule generic and name the GPIO
helpers as the example in the same sentence. I will take your wording
and add the line that clocks, resets and phys follow the same 'parse
now, request later' shape even though those helpers do not exist yet.
> a few lines down (around line 789) the existing text says the opposite
I missed that paragraph when I added mine above it. It describes
exactly the pattern this series removes, so v2 deletes it in the same
patch, keeping only the first sentence (that of_to_plat() must not
probe the device itself) since it is still true.
Regards,
Mehmet
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 4/4] test: dm: check the fixed regulator claims its GPIO at probe time
2026-08-28 10:52 [PATCH 0/4] dm: gpio: read GPIOs in of_to_plat(), request them in probe() Mehmet Fide
` (2 preceding siblings ...)
2026-08-28 10:52 ` [PATCH 3/4] doc: driver-model: state that of_to_plat() must not probe or claim Mehmet Fide
@ 2026-08-28 10:52 ` Mehmet Fide
2026-09-10 14:09 ` Simon Glass
2026-09-10 14:10 ` [0/4] dm: gpio: read GPIOs in of_to_plat(), request them in probe() Simon Glass
4 siblings, 1 reply; 16+ messages in thread
From: Mehmet Fide @ 2026-08-28 10:52 UTC (permalink / raw)
To: Simon Glass
Cc: Tom Rini, Jaehoon Chung, Peng Fan, Vincent Jardin, Ye Li,
Michal Simek, Aristo Chen, u-boot, Mehmet Fide
From: Mehmet Fide <mehmet.fide@screeningeagle.com>
Give sandbox a fixed regulator with an enable GPIO and check the two
phases explicitly: after of_to_plat() the GPIO is still unclaimed, and
only probe() requests it and sets the direction; enabling and disabling
the regulator then moves the pin.
Signed-off-by: Mehmet Fide <mehmet.fide@screeningeagle.com>
---
arch/sandbox/dts/test.dts | 9 +++++++++
test/dm/regulator.c | 29 +++++++++++++++++++++++++++++
2 files changed, 38 insertions(+)
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index d24feec5422..19773305b4c 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -880,6 +880,15 @@
compatible = "sandbox,fpga";
};
+ fixed_gpio_reg: regulator-fixed {
+ compatible = "regulator-fixed";
+ regulator-name = "fixed-gpio-enabled";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ enable-active-high;
+ gpio = <&gpio_a 10>;
+ };
+
pinctrl-gpio {
compatible = "sandbox,pinctrl-gpio";
diff --git a/test/dm/regulator.c b/test/dm/regulator.c
index 51007d4079d..b78627023af 100644
--- a/test/dm/regulator.c
+++ b/test/dm/regulator.c
@@ -12,6 +12,7 @@
#include <log.h>
#include <malloc.h>
#include <dm/device-internal.h>
+#include <asm/gpio.h>
#include <dm/root.h>
#include <dm/util.h>
#include <dm/test.h>
@@ -195,6 +196,34 @@ static int dm_test_power_regulator_set_get_current(struct unit_test_state *uts)
}
DM_TEST(dm_test_power_regulator_set_get_current, UTF_SCAN_FDT);
+/* The fixed regulator must claim its enable GPIO in probe, not before */
+static int dm_test_power_regulator_fixed_enable_gpio(struct unit_test_state *uts)
+{
+ struct gpio_desc chk;
+ struct udevice *dev;
+
+ ut_assertok(uclass_find_device_by_name(UCLASS_REGULATOR,
+ "regulator-fixed", &dev));
+ ut_assertok(device_of_to_plat(dev));
+ ut_assertok(dm_gpio_lookup_name("a10", &chk));
+
+ /* reading the platform data must not have claimed the GPIO */
+ ut_asserteq(GPIOF_UNUSED, gpio_get_function(chk.dev, chk.offset,
+ NULL));
+
+ ut_assertok(device_probe(dev));
+ ut_asserteq(GPIOF_OUTPUT, gpio_get_function(chk.dev, chk.offset,
+ NULL));
+
+ ut_assertok(regulator_set_enable(dev, true));
+ ut_asserteq(1, sandbox_gpio_get_value(chk.dev, chk.offset));
+ ut_assertok(regulator_set_enable(dev, false));
+ ut_asserteq(0, sandbox_gpio_get_value(chk.dev, chk.offset));
+
+ return 0;
+}
+DM_TEST(dm_test_power_regulator_fixed_enable_gpio, UTF_SCAN_FDT);
+
/* Test regulator set and get Enable method */
static int dm_test_power_regulator_set_get_enable(struct unit_test_state *uts)
{
--
2.54.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 4/4] test: dm: check the fixed regulator claims its GPIO at probe time
2026-08-28 10:52 ` [PATCH 4/4] test: dm: check the fixed regulator claims its GPIO at probe time Mehmet Fide
@ 2026-09-10 14:09 ` Simon Glass
2026-09-10 14:58 ` Mehmet Fide
0 siblings, 1 reply; 16+ messages in thread
From: Simon Glass @ 2026-09-10 14:09 UTC (permalink / raw)
To: mehmet.fide
Cc: Simon Glass, Tom Rini, Jaehoon Chung, Peng Fan, Vincent Jardin,
Ye Li, Michal Simek, Aristo Chen, u-boot, Mehmet Fide
Hi Mehmet,
On 2026-08-28T10:52:07, Mehmet Fide <mehmet.fide@gmail.com> wrote:
> test: dm: check the fixed regulator claims its GPIO at probe time
>
> Give sandbox a fixed regulator with an enable GPIO and check the two
> phases explicitly: after of_to_plat() the GPIO is still unclaimed, and
> only probe() requests it and sets the direction; enabling and disabling
> the regulator then moves the pin.
>
> Signed-off-by: Mehmet Fide <mehmet.fide@screeningeagle.com>
>
> arch/sandbox/dts/test.dts | 9 +++++++++
> test/dm/regulator.c | 29 +++++++++++++++++++++++++++++
> 2 files changed, 38 insertions(+)
> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> @@ -883,6 +883,15 @@
> + fixed_gpio_reg: regulator-fixed {
> + compatible = "regulator-fixed";
> + regulator-name = "fixed-gpio-enabled";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + enable-active-high;
> + gpio = <&gpio_a 10>;
> + };
Pin a10 is already used by hog_input_active_low further down in this
file. It happens to work because the test framework binds but does not
auto-probe DM_FLAG_PROBE_AFTER_BIND devices, so the hog never claims
the pin - but that is a subtle dependency a reader cannot see. Please
pick a pin not otherwise referenced in test.dts (a15-a18 look free),
or extend the bank and use a fresh offset.
> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> @@ -883,6 +883,15 @@
> + fixed_gpio_reg: regulator-fixed {
> + compatible = "regulator-fixed";
The node name regulator-fixed matches the compatible and is what
uclass_find_device_by_name() then keys on. If anyone adds a second
fixed regulator to test.dts the lookup becomes ambiguous. Please give
the node a distinctive name (e.g. fixed-gpio-reg matching the label)
so the test is not accidentally tied to being the only regulator-fixed
instance.
> diff --git a/test/dm/regulator.c b/test/dm/regulator.c
> @@ -195,6 +196,34 @@ static int dm_test_power_regulator_set_get_current(struct unit_test_state *uts)
> +/* The fixed regulator must claim its enable GPIO in probe, not before */
> +static int dm_test_power_regulator_fixed_enable_gpio(struct unit_test_state *uts)
Patch 2 moves the same claim-in-probe pattern into gpio-regulator and
fixed-clock, but only fixed-regulator is covered here. Please add
analogous cases (or parametrise this one) so a future regression in
either of the other two drivers is caught.
> diff --git a/test/dm/regulator.c b/test/dm/regulator.c
> @@ -195,6 +196,34 @@ static int dm_test_power_regulator_set_get_current(struct unit_test_state *uts)
> + ut_assertok(uclass_find_device_by_name(UCLASS_REGULATOR,
> + "regulator-fixed", &dev));
> + ut_assertok(device_of_to_plat(dev));
> + ut_assertok(dm_gpio_lookup_name("a10", &chk));
> +
> + /* reading the platform data must not have claimed the GPIO */
> + ut_asserteq(GPIOF_UNUSED, gpio_get_function(chk.dev, chk.offset,
> + NULL));
gpio_get_function() returns GPIOF_UNUSED whenever the pin is not
claimed via the DM name table, regardless of the sandbox pad state. So
this assertion only proves 'no dm_gpio_request() ran', not that the
direction was untouched. Would it be worth checking that on sandbox
the pad-level value/direction is what pinctrl (or the reset default)
set?
Regards,
Simon
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 4/4] test: dm: check the fixed regulator claims its GPIO at probe time
2026-09-10 14:09 ` Simon Glass
@ 2026-09-10 14:58 ` Mehmet Fide
0 siblings, 0 replies; 16+ messages in thread
From: Mehmet Fide @ 2026-09-10 14:58 UTC (permalink / raw)
To: Simon Glass
Cc: Tom Rini, Jaehoon Chung, Peng Fan, Vincent Jardin, Ye Li,
Michal Simek, Aristo Chen, u-boot, Mehmet Fide
Hi Simon,
On 2026-09-10 Simon Glass wrote:
> Pin a10 is already used by hog_input_active_low further down in this
> file.
I picked a10 after grepping for '&gpio_a 10' and the hogs address their
pins by bare number, so I missed it. Bank a has no free offset at all:
all 25 are taken between the phandle users and the hogs on 10 to 13,
and a15 to a18 are in use as well. Rather than grow the bank (three
gpio tests assert its size), v2 moves the regulator to c8 in the
pinmux-gpios bank, whose two cells also let the node carry the flags
the way a real regulator-fixed does.
> The node name regulator-fixed matches the compatible
Will rename it fixed-gpio-reg and look it up under that name.
> Patch 2 moves the same claim-in-probe pattern into gpio-regulator and
> fixed-clock, but only fixed-regulator is covered here.
fixed-clock drops out of patch 2 (see that thread). For gpio-regulator
v2 adds a node with two states on c9 and a test that mirrors this one:
unclaimed after of_to_plat(), output after probe(), voltage switching
the pin.
> gpio_get_function() returns GPIOF_UNUSED whenever the pin is not
> claimed via the DM name table, regardless of the sandbox pad state.
Good point. v2 also asserts sandbox_gpio_get_direction() is still input
after of_to_plat() and becomes output only after probe(), so the test
watches the pad and not just the name table.
Regards,
Mehmet
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [0/4] dm: gpio: read GPIOs in of_to_plat(), request them in probe()
2026-08-28 10:52 [PATCH 0/4] dm: gpio: read GPIOs in of_to_plat(), request them in probe() Mehmet Fide
` (3 preceding siblings ...)
2026-08-28 10:52 ` [PATCH 4/4] test: dm: check the fixed regulator claims its GPIO at probe time Mehmet Fide
@ 2026-09-10 14:10 ` Simon Glass
2026-09-10 14:58 ` [PATCH 0/4] " Mehmet Fide
4 siblings, 1 reply; 16+ messages in thread
From: Simon Glass @ 2026-09-10 14:10 UTC (permalink / raw)
To: mehmet.fide; +Cc: u-boot
Hi Mehmet,
On 2026-08-28T10:52:07, Mehmet Fide <mehmet.fide@gmail.com> wrote:
> This is the rework Simon asked for on "dm: core: read the device tree
> into plat data after pinctrl" [1], which is withdrawn: moving the
> of_to_plat() call around was the wrong fix, the real offender is
> claiming GPIOs while platform data is being read.
Thanks for taking this on!
I wonder how many other drivers probe in of_to_plat() ? It would be
interested to get the scale of the problem, if you can.
Regards,
Simon
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 0/4] dm: gpio: read GPIOs in of_to_plat(), request them in probe()
2026-09-10 14:10 ` [0/4] dm: gpio: read GPIOs in of_to_plat(), request them in probe() Simon Glass
@ 2026-09-10 14:58 ` Mehmet Fide
2026-09-10 15:08 ` Simon Glass
0 siblings, 1 reply; 16+ messages in thread
From: Mehmet Fide @ 2026-09-10 14:58 UTC (permalink / raw)
To: Simon Glass
Cc: Tom Rini, Jaehoon Chung, Peng Fan, Vincent Jardin, Ye Li,
Michal Simek, Aristo Chen, u-boot, Mehmet Fide
Hi Simon,
On 2026-09-10 Simon Glass wrote:
> I wonder how many other drivers probe in of_to_plat() ? It would be
> interested to get the scale of the problem, if you can.
I grepped next for gpio_request_by_name(), gpio_request_list_by_name()
and dm_gpio_request() called from an of_to_plat() method: 50 driver
files do it. By subsystem: video 22 (panels and backlights), net 6,
mmc 5, spi 4, usb 3 (ehci-vf among them, which is where I started),
pci 2, i2c 2, power 2 (the regulators in this series), w1, tpm, sound
and misc one each.
Most of them claim reset or enable lines that they then drive from
probe(), so they would convert the same way the regulators do here:
parse in of_to_plat(), request in probe(). I can only test two of
them: fec_mxc and fsl_esdhc_imx run on the Colibri VF50 I have, and I
will convert those in a follow-up series once the helper is settled.
I would rather not touch the other drivers,
the panels in particular, without hardware to check that moving the
claim out of of_to_plat() does not change what they power up and
when. I can post the list so their owners can pick them up.
I will answer the per-patch comments in their threads and send v2 once
those are settled.
Regards,
Mehmet
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/4] dm: gpio: read GPIOs in of_to_plat(), request them in probe()
2026-09-10 14:58 ` [PATCH 0/4] " Mehmet Fide
@ 2026-09-10 15:08 ` Simon Glass
0 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2026-09-10 15:08 UTC (permalink / raw)
To: Mehmet Fide
Cc: Tom Rini, Jaehoon Chung, Peng Fan, Vincent Jardin, Ye Li,
Michal Simek, Aristo Chen, u-boot, Mehmet Fide
Hi Mehmet,
On Thu, 10 Sept 2026 at 08:58, Mehmet Fide <mehmet.fide@gmail.com> wrote:
>
> Hi Simon,
>
> On 2026-09-10 Simon Glass wrote:
> > I wonder how many other drivers probe in of_to_plat() ? It would be
> > interested to get the scale of the problem, if you can.
>
> I grepped next for gpio_request_by_name(), gpio_request_list_by_name()
> and dm_gpio_request() called from an of_to_plat() method: 50 driver
> files do it. By subsystem: video 22 (panels and backlights), net 6,
> mmc 5, spi 4, usb 3 (ehci-vf among them, which is where I started),
> pci 2, i2c 2, power 2 (the regulators in this series), w1, tpm, sound
> and misc one each.
OK, not that much then.
>
> Most of them claim reset or enable lines that they then drive from
> probe(), so they would convert the same way the regulators do here:
> parse in of_to_plat(), request in probe(). I can only test two of
> them: fec_mxc and fsl_esdhc_imx run on the Colibri VF50 I have, and I
> will convert those in a follow-up series once the helper is settled.
> I would rather not touch the other drivers,
> the panels in particular, without hardware to check that moving the
> claim out of of_to_plat() does not change what they power up and
> when. I can post the list so their owners can pick them up.
Yes let's leave that to others one this series lands.
>
> I will answer the per-patch comments in their threads and send v2 once
> those are settled.
OK thanks - I didn't see anything in your replies that needs my input,
but LMK if I missed something.
Regards,
Simon
^ permalink raw reply [flat|nested] 16+ messages in thread