On Friday 24 April 2020 12:05:29 Rob Herring wrote:
> On Fri, Apr 24, 2020 at 10:39 AM Pali Rohár <pali@xxxxxxxxxx> wrote:
> > @@ -1046,6 +1071,22 @@ static int advk_pcie_probe(struct platform_device *pdev)
> > }
> > pcie->root_bus_nr = bus->start;
> >
> > + pcie->reset_gpio = devm_gpiod_get_from_of_node(dev, dev->of_node,
> > + "reset-gpios", 0,
> > + GPIOD_OUT_LOW,
> > + "pcie1-reset");
> > + ret = PTR_ERR_OR_ZERO(pcie->reset_gpio);
> > + if (ret) {
> > + if (ret == -ENOENT) {
> > + pcie->reset_gpio = NULL;
> > + } else {
> > + if (ret != -EPROBE_DEFER)
> > + dev_err(dev, "Failed to get reset-gpio: %i\n",
> > + ret);
> > + return ret;
> > + }
> > + }
>
> I believe all this can be replaced with devm_gpiod_get_optional.
I'm looking at the devm_gpiod_get_optional() code and it calls
fwnode_gpiod_get_index() which mangle passed gpio name by appending
suffix from array gpio_suffixes[]. And then mangled name is passed to
fwnode_get_named_gpiod(). At the end devm_gpiod_get_optional() handles
-ENOENT and transform it to NULL.
So via devm_gpiod_get_optional() I cannot request for "reset-gpios" DT
property directly, but I need to specify implicit name "reset" due to
appending suffix from gpio_suffixes[] array. And the only benefit is
that I do not need to handle ret == -ENOENT state.
Handling of -EPROBE_DEFER is still needed in aardvark driver.
For me it makes usage of devm_gpiod_get_optional() in this case harder
to debug code in future. As I would not be able to find a place which
reads 'reset-gpios' DT property via 'git grep reset-gpios'.
So it is really useful for this particular case to use
devm_gpiod_get_optional() instead of devm_gpiod_get_from_of_node().
