* [PATCH v2 0/4] dm: gpio: read GPIOs in of_to_plat(), request them in probe()
@ 2026-09-10 22:08 Mehmet Fide
2026-09-10 22:08 ` [PATCH v2 1/4] gpio: add a way to parse a GPIO now and request it later Mehmet Fide
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Mehmet Fide @ 2026-09-10 22:08 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, but the
only way to pick a GPIO out of it, gpio_request_by_name(), also probes
the controller and claims the pin. Several drivers therefore claim
GPIOs while their platform data is being read, before their own pinctrl
state is applied. On SoCs where the direction lives in the pad register
the pinctrl then undoes the direction the early claim set; on Vybrid
this leaves a fixed regulator configured as always-on powering nothing,
which is where this started (the ehci-vf VBUS supply).
Patch 1 splits gpio_request_by_name() in two: gpio_parse_by_name()
reads the phandle into a struct gpio_dt_spec that can live in the
platform data, gpio_request_parsed() claims it later from probe().
Patch 2 converts the fixed and gpio regulators, the two drivers that
share regulator_common, patch 3 writes the rule into the driver model
design document, and patch 4 checks the two phases in sandbox for both
regulators.
The remaining drivers that claim GPIOs from of_to_plat() (about fifty,
mostly panels and backlights) are left for their owners once the shape
is settled; I will convert fec_mxc and fsl_esdhc_imx, which I can test
on a Colibri VF50, in a follow-up.
Tested with 'ut dm' on sandbox against the same tree without the series
(the three new tests pass, the pre-existing failures are unchanged),
built for colibri_vf, evb-rk3399 and the sandbox variants, and booted
on a Colibri VF61 from NAND. There the VBUS regulator's pad register
(0x4004814c) reads 0x60 until the regulator probes and 0x22ef after,
the pin shows up as 'regulator-usbh-vbus.gpio' in 'gpio status', and
Linux boots from the same U-Boot.
Changes in v2:
- keep the list index in the spec so the request label matches
gpio_request_by_name(); rename gpio_dt_desc to gpio_dt_spec; document
the list_name lifetime and the -ENOENT contract, and use it instead of
present-flag guards in the regulators (Simon Glass)
- drop the GPIO request from the fixed-clock regulator (Simon Glass)
- doc: reword the helper sentence and remove the paragraph that allowed
probing providers from of_to_plat() (Simon Glass)
- test: move to the pinmux-gpios bank (a10 is a hog pin), name the node
fixed-gpio-reg, add a gpio regulator test, check the simulated pad
direction as well as the name table (Simon Glass)
Mehmet Fide (4):
gpio: add a way to parse a GPIO now and request it later
regulator: claim the enable GPIO at probe time, not in of_to_plat()
doc: driver-model: state that of_to_plat() must not probe or claim
test: dm: check the regulators claim their GPIOs at probe time
arch/sandbox/dts/test.dts | 18 +++++++
configs/sandbox64_defconfig | 1 +
configs/sandbox_defconfig | 1 +
configs/sandbox_flattree_defconfig | 1 +
configs/sandbox_noinst_defconfig | 1 +
configs/sandbox_spl_defconfig | 1 +
configs/sandbox_vpl_defconfig | 1 +
doc/develop/driver-model/design.rst | 16 ++++--
drivers/gpio/gpio-uclass.c | 34 ++++++++++++
drivers/power/regulator/fixed.c | 6 +++
drivers/power/regulator/gpio-regulator.c | 21 ++++++--
drivers/power/regulator/regulator_common.c | 26 ++++++++--
drivers/power/regulator/regulator_common.h | 3 ++
include/asm-generic/gpio.h | 60 ++++++++++++++++++++++
test/dm/gpio.c | 39 ++++++++++++++
test/dm/regulator.c | 58 +++++++++++++++++++++
16 files changed, 274 insertions(+), 13 deletions(-)
base-commit: d4152edb50356338af994d1c0483f65b717b4c15
--
2.55.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/4] gpio: add a way to parse a GPIO now and request it later
2026-09-10 22:08 [PATCH v2 0/4] dm: gpio: read GPIOs in of_to_plat(), request them in probe() Mehmet Fide
@ 2026-09-10 22:08 ` Mehmet Fide
2026-09-10 22:08 ` [PATCH v2 2/4] regulator: claim the enable GPIO at probe time, not in of_to_plat() Mehmet Fide
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Mehmet Fide @ 2026-09-10 22:08 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_spec without touching any device, so it can live in the
platform data; gpio_request_parsed() resolves the controller, claims
the GPIO under the same label gpio_request_by_name() would have used,
and applies the direction flags, for use in probe(). A spec whose
property was missing requests as -ENOENT on purpose, so a caller with
an optional GPIO does not need to check it first.
Signed-off-by: Mehmet Fide <mehmet.fide@screeningeagle.com>
---
Notes:
Changes in v2:
- keep the list index in the spec so the request label matches
gpio_request_by_name() (Simon Glass)
- rename struct gpio_dt_desc to gpio_dt_spec (Simon Glass)
- document the list_name lifetime and the deliberate -ENOENT (Simon Glass)
- test: check the pin is unclaimed between parse and request, and the
request label (Simon Glass)
drivers/gpio/gpio-uclass.c | 34 +++++++++++++++++++++
include/asm-generic/gpio.h | 60 ++++++++++++++++++++++++++++++++++++++
test/dm/gpio.c | 39 +++++++++++++++++++++++++
3 files changed, 133 insertions(+)
diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
index ff17cabd601..33eaf2a2022 100644
--- a/drivers/gpio/gpio-uclass.c
+++ b/drivers/gpio/gpio-uclass.c
@@ -1219,6 +1219,40 @@ 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_spec *spec)
+{
+ int ret;
+
+ spec->present = false;
+ ret = dev_read_phandle_with_args(dev, list_name, "#gpio-cells", 0,
+ index, &spec->args);
+ if (ret)
+ return ret;
+ spec->list_name = list_name;
+ spec->index = index;
+ spec->flags = flags;
+ spec->present = true;
+
+ return 0;
+}
+
+int gpio_request_parsed(struct udevice *dev, const struct gpio_dt_spec *spec,
+ struct gpio_desc *desc)
+{
+ struct ofnode_phandle_args args;
+
+ if (!spec->present) {
+ gpio_desc_init(desc, NULL, 0);
+ return -ENOENT;
+ }
+
+ args = spec->args;
+ return gpio_request_tail(0, ofnode_get_name(dev_ofnode(dev)), &args,
+ spec->list_name, spec->index, desc, spec->flags,
+ spec->index > 0, 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..8229a5faac1 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -574,6 +574,66 @@ 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_spec - devicetree specification 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. Only the
+ * pointer is kept, so the string must outlive the spec; callers pass
+ * string literals
+ * @index: index of the GPIO in the property, kept for the request label
+ * @flags: GPIOD_... flags requested by the caller
+ * @present: true if the property exists and was parsed
+ */
+struct gpio_dt_spec {
+ struct ofnode_phandle_args args;
+ const char *list_name;
+ int index;
+ 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
+ * @spec: Returns the parsed specification
+ * 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_spec *spec);
+
+/**
+ * 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. The request label is the same one gpio_request_by_name()
+ * would have used.
+ *
+ * A spec whose property was missing is deliberately accepted and answered
+ * with -ENOENT, with @desc left invalid, so a caller with an optional GPIO
+ * need not check @present first and can simply tolerate -ENOENT.
+ *
+ * @dev: Device requesting the GPIO (used for the request label)
+ * @spec: Specification returned by gpio_parse_by_name()
+ * @desc: Returns the GPIO description, ready for use
+ * Return: 0 if OK, -ENOENT if @spec holds no GPIO, other -ve on error
+ */
+int gpio_request_parsed(struct udevice *dev, const struct gpio_dt_spec *spec,
+ 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 3d966e0d1a6..2401c5ed1be 100644
--- a/test/dm/gpio.c
+++ b/test/dm/gpio.c
@@ -257,6 +257,45 @@ 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_spec spec;
+ struct gpio_desc desc, chk;
+ struct udevice *dev;
+ const char *label;
+
+ ut_assertok(uclass_get_device(UCLASS_TEST_FDT, 0, &dev));
+ ut_asserteq_str("a-test", dev->name);
+
+ /* test2-gpios index 1 is a4: parsing alone must not claim it */
+ ut_assertok(dm_gpio_lookup_name("a4", &chk));
+ ut_assertok(gpio_parse_by_name(dev, "test2-gpios", 1, GPIOD_IS_OUT,
+ &spec));
+ ut_asserteq(true, spec.present);
+ ut_asserteq(GPIOF_UNUSED, gpio_get_function(chk.dev, chk.offset,
+ NULL));
+ ut_asserteq(0, sandbox_gpio_get_direction(chk.dev, chk.offset));
+
+ /* the request claims it with the label gpio_request_by_name() uses */
+ ut_assertok(gpio_request_parsed(dev, &spec, &desc));
+ ut_asserteq_ptr(chk.dev, desc.dev);
+ ut_asserteq(chk.offset, desc.offset);
+ ut_asserteq(GPIOF_OUTPUT, gpio_get_function(desc.dev, desc.offset,
+ &label));
+ ut_asserteq_str("a-test.test2-gpios1", label);
+ 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, &spec));
+ ut_asserteq(-ENOENT, gpio_request_parsed(dev, &spec, &desc));
+ ut_asserteq(false, dm_gpio_is_valid(&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)
{
--
2.55.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/4] regulator: claim the enable GPIO at probe time, not in of_to_plat()
2026-09-10 22:08 [PATCH v2 0/4] dm: gpio: read GPIOs in of_to_plat(), request them in probe() Mehmet Fide
2026-09-10 22:08 ` [PATCH v2 1/4] gpio: add a way to parse a GPIO now and request it later Mehmet Fide
@ 2026-09-10 22:08 ` Mehmet Fide
2026-09-10 22:08 ` [PATCH v2 3/4] doc: driver-model: state that of_to_plat() must not probe or claim Mehmet Fide
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Mehmet Fide @ 2026-09-10 22:08 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 regulator's
probe method. The gpio regulator's voltage-control GPIO has the same
problem and moves the same way.
Two things change on the way. The gpio regulator used to log and carry
on when its voltage GPIO could not be claimed; it now fails to probe for
anything but a missing property, since it cannot switch anything
without that pin. The fixed regulator already returned such errors from
of_to_plat(), so nothing changes there. And the parsed specification
stays in the platform data after probe: priv is not allocated when
of_to_plat() runs, so plat is the only place it can live, a few dozen
bytes per regulator traded for keeping the two phases simple.
The fixed-clock regulator shares the of_to_plat() and used to claim the
GPIO too, although its set_enable() only drives the clock; no
devicetree in the tree gives it one, so it no longer requests it.
Signed-off-by: Mehmet Fide <mehmet.fide@screeningeagle.com>
---
Notes:
Changes in v2:
- rely on the -ENOENT contract instead of checking the present flag in
both probe methods (Simon Glass)
- drop the request from the fixed-clock regulator, no devicetree uses
it (Simon Glass)
- reword the stale debug message in gpio-regulator (Simon Glass)
- commit message: spell out the gpio-regulator behaviour change and the
plat footprint (Simon Glass)
drivers/power/regulator/fixed.c | 6 +++++
drivers/power/regulator/gpio-regulator.c | 21 +++++++++++++----
drivers/power/regulator/regulator_common.c | 26 ++++++++++++++++++----
drivers/power/regulator/regulator_common.h | 3 +++
4 files changed, 48 insertions(+), 8 deletions(-)
diff --git a/drivers/power/regulator/fixed.c b/drivers/power/regulator/fixed.c
index 1dd137f493e..ff01812e8d0 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;
@@ -150,6 +155,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..84a61f1a92c 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_spec 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,10 +47,10 @@ 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);
+ debug("gpio-regulator: cannot parse the voltage GPIO: %d\n", ret);
ret = dev_read_size(dev, "states");
if (ret < 0)
@@ -76,6 +76,18 @@ 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;
+
+ ret = gpio_request_parsed(dev, &plat->gpio_dt, &plat->gpio);
+ if (ret && ret != -ENOENT)
+ 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 +165,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..dfc58cd60bf 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,25 @@ 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))
+ return 0;
+
+ /* the enable GPIO is optional: -ENOENT means there is none */
+ ret = gpio_request_parsed(dev, &plat->gpio_dt, &plat->gpio);
+ if (ret == -ENOENT)
+ return 0;
+ if (ret)
+ debug("Regulator '%s' enable GPIO request failed: %d\n",
+ dev->name, ret);
+
+ return ret;
+}
+
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..951a02b5416 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_spec 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.55.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 3/4] doc: driver-model: state that of_to_plat() must not probe or claim
2026-09-10 22:08 [PATCH v2 0/4] dm: gpio: read GPIOs in of_to_plat(), request them in probe() Mehmet Fide
2026-09-10 22:08 ` [PATCH v2 1/4] gpio: add a way to parse a GPIO now and request it later Mehmet Fide
2026-09-10 22:08 ` [PATCH v2 2/4] regulator: claim the enable GPIO at probe time, not in of_to_plat() Mehmet Fide
@ 2026-09-10 22:08 ` Mehmet Fide
2026-09-10 22:08 ` [PATCH v2 4/4] test: dm: check the regulators claim their GPIOs at probe time Mehmet Fide
2026-09-29 22:45 ` [PATCH v2 0/4] dm: gpio: read GPIOs in of_to_plat(), request them in probe() Tom Rini
4 siblings, 0 replies; 6+ messages in thread
From: Mehmet Fide @ 2026-09-10 22:08 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, the
same shape clocks, resets and phys will want.
The paragraph a few lines further down allowed exactly the opposite,
probing GPIO, clock and reset providers from of_to_plat() to select a
resource. That is the pattern the helpers exist to remove, so drop it;
only its first sentence, that of_to_plat() must not probe the device
itself, is still true and stays.
Signed-off-by: Mehmet Fide <mehmet.fide@screeningeagle.com>
---
Notes:
Changes in v2:
- reword the sentence about the GPIO helpers, mention clocks, resets and
phys (Simon Glass)
- remove the paragraph that allowed probing providers from of_to_plat()
(Simon Glass)
doc/develop/driver-model/design.rst | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/doc/develop/driver-model/design.rst b/doc/develop/driver-model/design.rst
index 633545944d1..4b1fe0839a7 100644
--- a/doc/develop/driver-model/design.rst
+++ b/doc/develop/driver-model/design.rst
@@ -759,6 +759,16 @@ 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 with
+gpio_parse_by_name() and claim it in probe() with gpio_request_parsed(). The
+same 'parse now, request later' shape applies to clocks, resets and phys,
+even though those helpers do not exist yet.
+
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
@@ -778,11 +788,7 @@ present will cause an error on probe, yet we still must tell Linux about
the SD card connector in case it is used while Linux is running.
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.
+the device itself.
If your device relies on its parent setting up a suitable address space, so
that dev_read_addr() works correctly, then make sure that the parent device
--
2.55.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 4/4] test: dm: check the regulators claim their GPIOs at probe time
2026-09-10 22:08 [PATCH v2 0/4] dm: gpio: read GPIOs in of_to_plat(), request them in probe() Mehmet Fide
` (2 preceding siblings ...)
2026-09-10 22:08 ` [PATCH v2 3/4] doc: driver-model: state that of_to_plat() must not probe or claim Mehmet Fide
@ 2026-09-10 22:08 ` Mehmet Fide
2026-09-29 22:45 ` [PATCH v2 0/4] dm: gpio: read GPIOs in of_to_plat(), request them in probe() Tom Rini
4 siblings, 0 replies; 6+ messages in thread
From: Mehmet Fide @ 2026-09-10 22:08 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 a gpio regulator
with a voltage GPIO and check the two phases explicitly: after
of_to_plat() the pin is unclaimed in the GPIO name table and its
simulated direction is still the input default; only probe() requests
it and turns it into an output; enabling the regulator, or switching
its voltage, then moves the pin.
The pins come from the pinmux-gpios bank, whose two cells also carry
the flags the way a real regulator node does; the base-gpios bank has
no offset left that the other tests or its hogs do not use. The gpio
regulator driver was not built for sandbox before.
Signed-off-by: Mehmet Fide <mehmet.fide@screeningeagle.com>
---
Notes:
Changes in v2:
- move the regulator to c8 in the pinmux-gpios bank, a10 is a hog pin
(Simon Glass)
- name the node fixed-gpio-reg instead of the compatible (Simon Glass)
- add a gpio regulator on c9 and a test for it, enable
CONFIG_DM_REGULATOR_GPIO in the sandbox defconfigs (Simon Glass)
- check the simulated pad direction, not only the name table (Simon Glass)
arch/sandbox/dts/test.dts | 18 ++++++++++
configs/sandbox64_defconfig | 1 +
configs/sandbox_defconfig | 1 +
configs/sandbox_flattree_defconfig | 1 +
configs/sandbox_noinst_defconfig | 1 +
configs/sandbox_spl_defconfig | 1 +
configs/sandbox_vpl_defconfig | 1 +
test/dm/regulator.c | 58 ++++++++++++++++++++++++++++++
8 files changed, 82 insertions(+)
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index a5d8d3977a0..b05f4c24c3a 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -883,6 +883,24 @@
compatible = "sandbox,fpga";
};
+ fixed_gpio_reg: fixed-gpio-reg {
+ compatible = "regulator-fixed";
+ regulator-name = "fixed-gpio-enabled";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ enable-active-high;
+ gpio = <&gpio_c 8 GPIO_ACTIVE_HIGH>;
+ };
+
+ gpio_reg: gpio-reg {
+ compatible = "regulator-gpio";
+ regulator-name = "gpio-switched";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <3300000>;
+ gpios = <&gpio_c 9 GPIO_ACTIVE_HIGH>;
+ states = <1800000 0>, <3300000 1>;
+ };
+
pinctrl-gpio {
compatible = "sandbox,pinctrl-gpio";
diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
index 070ef0b6369..59e08d03f77 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -220,6 +220,7 @@ CONFIG_DM_REGULATOR=y
CONFIG_REGULATOR_ACT8846=y
CONFIG_DM_REGULATOR_MAX77686=y
CONFIG_DM_REGULATOR_FIXED=y
+CONFIG_DM_REGULATOR_GPIO=y
CONFIG_REGULATOR_RK8XX=y
CONFIG_REGULATOR_S5M8767=y
CONFIG_DM_REGULATOR_SANDBOX=y
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index c6edd9d2423..5ebbe5ca8f6 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -308,6 +308,7 @@ CONFIG_DM_REGULATOR=y
CONFIG_REGULATOR_ACT8846=y
CONFIG_DM_REGULATOR_MAX77686=y
CONFIG_DM_REGULATOR_FIXED=y
+CONFIG_DM_REGULATOR_GPIO=y
CONFIG_REGULATOR_RK8XX=y
CONFIG_REGULATOR_S5M8767=y
CONFIG_DM_REGULATOR_SANDBOX=y
diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig
index 9546fbf730f..57f915d4ba9 100644
--- a/configs/sandbox_flattree_defconfig
+++ b/configs/sandbox_flattree_defconfig
@@ -175,6 +175,7 @@ CONFIG_DM_REGULATOR=y
CONFIG_REGULATOR_ACT8846=y
CONFIG_DM_REGULATOR_MAX77686=y
CONFIG_DM_REGULATOR_FIXED=y
+CONFIG_DM_REGULATOR_GPIO=y
CONFIG_REGULATOR_S5M8767=y
CONFIG_DM_REGULATOR_SANDBOX=y
CONFIG_REGULATOR_TPS65090=y
diff --git a/configs/sandbox_noinst_defconfig b/configs/sandbox_noinst_defconfig
index 0708b65ea80..7a755012795 100644
--- a/configs/sandbox_noinst_defconfig
+++ b/configs/sandbox_noinst_defconfig
@@ -219,6 +219,7 @@ CONFIG_DM_REGULATOR=y
CONFIG_REGULATOR_ACT8846=y
CONFIG_DM_REGULATOR_MAX77686=y
CONFIG_DM_REGULATOR_FIXED=y
+CONFIG_DM_REGULATOR_GPIO=y
CONFIG_REGULATOR_RK8XX=y
CONFIG_REGULATOR_S5M8767=y
CONFIG_DM_REGULATOR_SANDBOX=y
diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig
index 11f16f5d622..60b497f4888 100644
--- a/configs/sandbox_spl_defconfig
+++ b/configs/sandbox_spl_defconfig
@@ -186,6 +186,7 @@ CONFIG_DM_REGULATOR=y
CONFIG_REGULATOR_ACT8846=y
CONFIG_DM_REGULATOR_MAX77686=y
CONFIG_DM_REGULATOR_FIXED=y
+CONFIG_DM_REGULATOR_GPIO=y
CONFIG_REGULATOR_RK8XX=y
CONFIG_REGULATOR_S5M8767=y
CONFIG_DM_REGULATOR_SANDBOX=y
diff --git a/configs/sandbox_vpl_defconfig b/configs/sandbox_vpl_defconfig
index c7df8571663..c85bfbfb3b1 100644
--- a/configs/sandbox_vpl_defconfig
+++ b/configs/sandbox_vpl_defconfig
@@ -193,6 +193,7 @@ CONFIG_DM_REGULATOR=y
CONFIG_REGULATOR_ACT8846=y
CONFIG_DM_REGULATOR_MAX77686=y
CONFIG_DM_REGULATOR_FIXED=y
+CONFIG_DM_REGULATOR_GPIO=y
CONFIG_REGULATOR_RK8XX=y
CONFIG_REGULATOR_S5M8767=y
CONFIG_DM_REGULATOR_SANDBOX=y
diff --git a/test/dm/regulator.c b/test/dm/regulator.c
index 51007d4079d..123cee6f499 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,63 @@ 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);
+/* Reading the platform data must leave the pin untouched, probe claims it */
+static int check_gpio_claimed_in_probe(struct unit_test_state *uts,
+ struct udevice *dev, struct gpio_desc *chk)
+{
+ ut_assertok(device_of_to_plat(dev));
+ ut_asserteq(GPIOF_UNUSED, gpio_get_function(chk->dev, chk->offset,
+ NULL));
+ ut_asserteq(0, sandbox_gpio_get_direction(chk->dev, chk->offset));
+
+ ut_assertok(device_probe(dev));
+ ut_asserteq(GPIOF_OUTPUT, gpio_get_function(chk->dev, chk->offset,
+ NULL));
+ ut_assert(sandbox_gpio_get_direction(chk->dev, chk->offset) > 0);
+
+ return 0;
+}
+
+/* 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,
+ "fixed-gpio-reg", &dev));
+ ut_assertok(dm_gpio_lookup_name("c8", &chk));
+ ut_assertok(check_gpio_claimed_in_probe(uts, dev, &chk));
+
+ 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);
+
+/* The gpio regulator must claim its voltage GPIO in probe, not before */
+static int dm_test_power_regulator_gpio_value_gpio(struct unit_test_state *uts)
+{
+ struct gpio_desc chk;
+ struct udevice *dev;
+
+ ut_assertok(uclass_find_device_by_name(UCLASS_REGULATOR, "gpio-reg",
+ &dev));
+ ut_assertok(dm_gpio_lookup_name("c9", &chk));
+ ut_assertok(check_gpio_claimed_in_probe(uts, dev, &chk));
+
+ ut_assertok(regulator_set_value(dev, 3300000));
+ ut_asserteq(1, sandbox_gpio_get_value(chk.dev, chk.offset));
+ ut_assertok(regulator_set_value(dev, 1800000));
+ ut_asserteq(0, sandbox_gpio_get_value(chk.dev, chk.offset));
+
+ return 0;
+}
+DM_TEST(dm_test_power_regulator_gpio_value_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.55.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 0/4] dm: gpio: read GPIOs in of_to_plat(), request them in probe()
2026-09-10 22:08 [PATCH v2 0/4] dm: gpio: read GPIOs in of_to_plat(), request them in probe() Mehmet Fide
` (3 preceding siblings ...)
2026-09-10 22:08 ` [PATCH v2 4/4] test: dm: check the regulators claim their GPIOs at probe time Mehmet Fide
@ 2026-09-29 22:45 ` Tom Rini
4 siblings, 0 replies; 6+ messages in thread
From: Tom Rini @ 2026-09-29 22:45 UTC (permalink / raw)
To: Simon Glass, Mehmet Fide
Cc: Jaehoon Chung, Peng Fan, Vincent Jardin, Ye Li, Michal Simek,
Aristo Chen, u-boot, Mehmet Fide
On Fri, 11 Sep 2026 00:08:14 +0200, Mehmet Fide wrote:
> From: Mehmet Fide <mehmet.fide@screeningeagle.com>
>
> A driver's of_to_plat() method must only read the devicetree, but the
> only way to pick a GPIO out of it, gpio_request_by_name(), also probes
> the controller and claims the pin. Several drivers therefore claim
> GPIOs while their platform data is being read, before their own pinctrl
> state is applied. On SoCs where the direction lives in the pad register
> the pinctrl then undoes the direction the early claim set; on Vybrid
> this leaves a fixed regulator configured as always-on powering nothing,
> which is where this started (the ehci-vf VBUS supply).
>
> [...]
Applied to u-boot/next, thanks!
[1/4] gpio: add a way to parse a GPIO now and request it later
commit: 4cf32565c3c44069e2015b309c02cd29bd590f7f
[2/4] regulator: claim the enable GPIO at probe time, not in of_to_plat()
commit: 61665f113d3704311dd60aa76394a3abb44b66af
[3/4] doc: driver-model: state that of_to_plat() must not probe or claim
commit: 6cca7e5498a7c5bc7b9d100a54b7778d3bc0c365
[4/4] test: dm: check the regulators claim their GPIOs at probe time
commit: a338b718373be8df70a6ef8bbafecd4d1c6c4d31
--
Tom
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-09-29 22:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-10 22:08 [PATCH v2 0/4] dm: gpio: read GPIOs in of_to_plat(), request them in probe() Mehmet Fide
2026-09-10 22:08 ` [PATCH v2 1/4] gpio: add a way to parse a GPIO now and request it later Mehmet Fide
2026-09-10 22:08 ` [PATCH v2 2/4] regulator: claim the enable GPIO at probe time, not in of_to_plat() Mehmet Fide
2026-09-10 22:08 ` [PATCH v2 3/4] doc: driver-model: state that of_to_plat() must not probe or claim Mehmet Fide
2026-09-10 22:08 ` [PATCH v2 4/4] test: dm: check the regulators claim their GPIOs at probe time Mehmet Fide
2026-09-29 22:45 ` [PATCH v2 0/4] dm: gpio: read GPIOs in of_to_plat(), request them in probe() Tom Rini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox