Hi Mehmet, On 2026-08-28T10:52:07, Mehmet Fide <[email protected]> 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 <[email protected]> > > 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
