[v2,5/6] gpiolib: consolidate GPIO lookups

Message ID 20221031-gpiolib-swnode-v2-5-81f55af5fa0e@gmail.com
State New
Headers
Series Add support for software nodes to gpiolib |

Commit Message

Dmitry Torokhov Nov. 9, 2022, 12:26 a.m. UTC
  Ensure that all paths to obtain/look up GPIOD from generic
consumer-visible APIs go through the new gpiod_find_and_request()
helper, so that we can easily extend it with support for new firmware
mechanisms.

The only exception is OF-specific [devm_]gpiod_get_from_of_node() API
that is still being used by a couple of drivers and will be removed as
soon as patches converting them to use generic fwnode/device APIs are
accepted.

Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/gpio/gpiolib-acpi.c |  39 ---------
 drivers/gpio/gpiolib-acpi.h |  10 ---
 drivers/gpio/gpiolib.c      | 207 +++++++++++++++++---------------------------
 3 files changed, 81 insertions(+), 175 deletions(-)
  

Comments

Andy Shevchenko Nov. 9, 2022, 11:25 a.m. UTC | #1
On Tue, Nov 08, 2022 at 04:26:50PM -0800, Dmitry Torokhov wrote:
> Ensure that all paths to obtain/look up GPIOD from generic
> consumer-visible APIs go through the new gpiod_find_and_request()
> helper, so that we can easily extend it with support for new firmware
> mechanisms.
> 
> The only exception is OF-specific [devm_]gpiod_get_from_of_node() API
> that is still being used by a couple of drivers and will be removed as
> soon as patches converting them to use generic fwnode/device APIs are
> accepted.

...

> +static struct gpio_desc *gpiod_find_by_fwnode(struct fwnode_handle *fwnode,
> +					      struct device *consumer,
> +					      const char *con_id,
> +					      unsigned int idx,
> +					      enum gpiod_flags *flags,
> +					      unsigned long *lookupflags)
>  {
> -	unsigned long lflags = GPIO_LOOKUP_FLAGS_DEFAULT;

> -	struct gpio_desc *desc = ERR_PTR(-ENODEV);

Not sure why this is needed. Now I see that else branch has been changed,
but looking closer to it, we can drop it completely, while leaving this
line untouched, correct?

> -	int ret;
> +	struct gpio_desc *desc;
>  
> +	dev_dbg(consumer, "GPIO lookup for consumer %s in node '%pfw'\n",
> +		con_id, fwnode);
> +
> +	/* Using device tree? */
>  	if (is_of_node(fwnode)) {
> -		desc = gpiod_get_from_of_node(to_of_node(fwnode),
> -					      propname, index,
> -					      dflags,
> -					      label);
> -		return desc;
> +		dev_dbg(consumer, "using device tree for GPIO lookup\n");
> +		desc = of_find_gpio(to_of_node(fwnode),
> +				    con_id, idx, lookupflags);

At least con_id can be placed on the previous line.

>  	} else if (is_acpi_node(fwnode)) {
> -		desc = acpi_node_get_gpiod(fwnode, propname, index,
> -					   &lflags, &dflags);
> -		if (IS_ERR(desc))
> -			return desc;
> +		dev_dbg(consumer, "using ACPI for GPIO lookup\n");
> +		desc = acpi_find_gpio(fwnode, con_id, idx, flags, lookupflags);
>  	} else {
> -		return ERR_PTR(-EINVAL);
> +		desc = ERR_PTR(-ENOENT);
>  	}
>  
> -	/* Currently only ACPI takes this path */
> +	return desc;
> +}

...

> +	struct gpio_desc *desc = ERR_PTR(-ENOENT);
> +	unsigned long lookupflags;
> +	int ret;

> +	if (!IS_ERR_OR_NULL(fwnode))

I think this is superfluous check.

Now in the form of this series, you have only a single dev_dbg() that tries to
dereference it. Do we really need to have it there, since every branch has its
own dev_dbg() anyway?

> +		desc = gpiod_find_by_fwnode(fwnode, consumer, con_id, idx,
> +					    &flags, &lookupflags);

> +

This blank line can be dropped after addressing above.

> +	if (gpiod_not_found(desc) && platform_lookup_allowed) {
> +		/*
> +		 * Either we are not using DT or ACPI, or their lookup did not
> +		 * return a result. In that case, use platform lookup as a
> +		 * fallback.
> +		 */
> +		dev_dbg(consumer, "using lookup tables for GPIO lookup\n");
> +		desc = gpiod_find(consumer, con_id, idx, &lookupflags);
> +	}
> +
> +	if (IS_ERR(desc)) {
> +		dev_dbg(consumer, "No GPIO consumer %s found\n", con_id);
> +		return desc;
> +	}

...

> +	return gpiod_find_and_request(NULL, fwnode, con_id, index, flags, label,
> +				      false);

One line?

...

> +	return gpiod_find_and_request(dev, fwnode, con_id, idx, flags, label,
> +				      true);

One line?
  
Dmitry Torokhov Nov. 9, 2022, 7 p.m. UTC | #2
On Wed, Nov 09, 2022 at 01:25:06PM +0200, Andy Shevchenko wrote:
> On Tue, Nov 08, 2022 at 04:26:50PM -0800, Dmitry Torokhov wrote:
> > Ensure that all paths to obtain/look up GPIOD from generic
> > consumer-visible APIs go through the new gpiod_find_and_request()
> > helper, so that we can easily extend it with support for new firmware
> > mechanisms.
> > 
> > The only exception is OF-specific [devm_]gpiod_get_from_of_node() API
> > that is still being used by a couple of drivers and will be removed as
> > soon as patches converting them to use generic fwnode/device APIs are
> > accepted.
> 
> ...
> 
> > +static struct gpio_desc *gpiod_find_by_fwnode(struct fwnode_handle *fwnode,
> > +					      struct device *consumer,
> > +					      const char *con_id,
> > +					      unsigned int idx,
> > +					      enum gpiod_flags *flags,
> > +					      unsigned long *lookupflags)
> >  {
> > -	unsigned long lflags = GPIO_LOOKUP_FLAGS_DEFAULT;
> 
> > -	struct gpio_desc *desc = ERR_PTR(-ENODEV);
> 
> Not sure why this is needed. Now I see that else branch has been changed,
> but looking closer to it, we can drop it completely, while leaving this
> line untouched, correct?

Yes. I believe removing an initializer and doing a series of if/else
if/else was discussed and [soft] agreed-on in the previous review cycle,
but I can change it back.

I think we still need to have it return -ENOENT and not -ENODEV/-EINVAL
so that we can fall back to GPIO lookup tables when dealing with an
unsupported node type.

> 
> > -	int ret;
> > +	struct gpio_desc *desc;
> >  
> > +	dev_dbg(consumer, "GPIO lookup for consumer %s in node '%pfw'\n",
> > +		con_id, fwnode);
> > +
> > +	/* Using device tree? */
> >  	if (is_of_node(fwnode)) {
> > -		desc = gpiod_get_from_of_node(to_of_node(fwnode),
> > -					      propname, index,
> > -					      dflags,
> > -					      label);
> > -		return desc;
> > +		dev_dbg(consumer, "using device tree for GPIO lookup\n");
> > +		desc = of_find_gpio(to_of_node(fwnode),
> > +				    con_id, idx, lookupflags);
> 
> At least con_id can be placed on the previous line.

OK, I made it all 1 line.

> 
> >  	} else if (is_acpi_node(fwnode)) {
> > -		desc = acpi_node_get_gpiod(fwnode, propname, index,
> > -					   &lflags, &dflags);
> > -		if (IS_ERR(desc))
> > -			return desc;
> > +		dev_dbg(consumer, "using ACPI for GPIO lookup\n");
> > +		desc = acpi_find_gpio(fwnode, con_id, idx, flags, lookupflags);
> >  	} else {
> > -		return ERR_PTR(-EINVAL);
> > +		desc = ERR_PTR(-ENOENT);
> >  	}
> >  
> > -	/* Currently only ACPI takes this path */
> > +	return desc;
> > +}
> 
> ...
> 
> > +	struct gpio_desc *desc = ERR_PTR(-ENOENT);
> > +	unsigned long lookupflags;
> > +	int ret;
> 
> > +	if (!IS_ERR_OR_NULL(fwnode))
> 
> I think this is superfluous check.
> 
> Now in the form of this series, you have only a single dev_dbg() that tries to
> dereference it. Do we really need to have it there, since every branch has its
> own dev_dbg() anyway?

As I mentioned, I like to keep this check to show the reader that we
should only descend into gpiod_find_by_fwnode() if we have a valid
fwnode. It is less about code generation and more about the intent.

I did change the logging to remove extra dev_dbg(). We will lose message
when dealing with unsupported node type, but that should not really
happen in practice.

> 
> > +		desc = gpiod_find_by_fwnode(fwnode, consumer, con_id, idx,
> > +					    &flags, &lookupflags);
> 
> > +
> 
> This blank line can be dropped after addressing above.
> 
> > +	if (gpiod_not_found(desc) && platform_lookup_allowed) {
> > +		/*
> > +		 * Either we are not using DT or ACPI, or their lookup did not
> > +		 * return a result. In that case, use platform lookup as a
> > +		 * fallback.
> > +		 */
> > +		dev_dbg(consumer, "using lookup tables for GPIO lookup\n");
> > +		desc = gpiod_find(consumer, con_id, idx, &lookupflags);
> > +	}
> > +
> > +	if (IS_ERR(desc)) {
> > +		dev_dbg(consumer, "No GPIO consumer %s found\n", con_id);
> > +		return desc;
> > +	}
> 
> ...
> 
> > +	return gpiod_find_and_request(NULL, fwnode, con_id, index, flags, label,
> > +				      false);
> 
> One line?

OK :)

> 
> ...
> 
> > +	return gpiod_find_and_request(dev, fwnode, con_id, idx, flags, label,
> > +				      true);
> 
> One line?

OK.

Thanks,
  
Andy Shevchenko Nov. 10, 2022, 1:42 p.m. UTC | #3
On Wed, Nov 09, 2022 at 11:00:29AM -0800, Dmitry Torokhov wrote:
> On Wed, Nov 09, 2022 at 01:25:06PM +0200, Andy Shevchenko wrote:
> > On Tue, Nov 08, 2022 at 04:26:50PM -0800, Dmitry Torokhov wrote:

...

> > > +static struct gpio_desc *gpiod_find_by_fwnode(struct fwnode_handle *fwnode,
> > > +					      struct device *consumer,
> > > +					      const char *con_id,
> > > +					      unsigned int idx,
> > > +					      enum gpiod_flags *flags,
> > > +					      unsigned long *lookupflags)
> > >  {
> > > -	unsigned long lflags = GPIO_LOOKUP_FLAGS_DEFAULT;
> > 
> > > -	struct gpio_desc *desc = ERR_PTR(-ENODEV);
> > 
> > Not sure why this is needed. Now I see that else branch has been changed,
> > but looking closer to it, we can drop it completely, while leaving this
> > line untouched, correct?
> 
> Yes. I believe removing an initializer and doing a series of if/else
> if/else was discussed and [soft] agreed-on in the previous review cycle,
> but I can change it back.
> 
> I think we still need to have it return -ENOENT and not -ENODEV/-EINVAL
> so that we can fall back to GPIO lookup tables when dealing with an
> unsupported node type.

Right, okay, let's go with whatever variant you find better.

...

> > > +	if (!IS_ERR_OR_NULL(fwnode))
> > 
> > I think this is superfluous check.
> > 
> > Now in the form of this series, you have only a single dev_dbg() that tries to
> > dereference it. Do we really need to have it there, since every branch has its
> > own dev_dbg() anyway?
> 
> As I mentioned, I like to keep this check to show the reader that we
> should only descend into gpiod_find_by_fwnode() if we have a valid
> fwnode. It is less about code generation and more about the intent.

Yes, but if fwnode is not found, we have a next check for that. I really don't
think we lose anything by dropping the check and gaining the code generation as
a side effect.
  
Dmitry Torokhov Nov. 10, 2022, 5:21 p.m. UTC | #4
On Thu, Nov 10, 2022 at 03:42:40PM +0200, Andy Shevchenko wrote:
> On Wed, Nov 09, 2022 at 11:00:29AM -0800, Dmitry Torokhov wrote:
> > On Wed, Nov 09, 2022 at 01:25:06PM +0200, Andy Shevchenko wrote:
> > > On Tue, Nov 08, 2022 at 04:26:50PM -0800, Dmitry Torokhov wrote:
> 
> ...
> 
> > > > +static struct gpio_desc *gpiod_find_by_fwnode(struct fwnode_handle *fwnode,
> > > > +					      struct device *consumer,
> > > > +					      const char *con_id,
> > > > +					      unsigned int idx,
> > > > +					      enum gpiod_flags *flags,
> > > > +					      unsigned long *lookupflags)
> > > >  {
> > > > -	unsigned long lflags = GPIO_LOOKUP_FLAGS_DEFAULT;
> > > 
> > > > -	struct gpio_desc *desc = ERR_PTR(-ENODEV);
> > > 
> > > Not sure why this is needed. Now I see that else branch has been changed,
> > > but looking closer to it, we can drop it completely, while leaving this
> > > line untouched, correct?
> > 
> > Yes. I believe removing an initializer and doing a series of if/else
> > if/else was discussed and [soft] agreed-on in the previous review cycle,
> > but I can change it back.
> > 
> > I think we still need to have it return -ENOENT and not -ENODEV/-EINVAL
> > so that we can fall back to GPIO lookup tables when dealing with an
> > unsupported node type.
> 
> Right, okay, let's go with whatever variant you find better.
> 
> ...
> 
> > > > +	if (!IS_ERR_OR_NULL(fwnode))
> > > 
> > > I think this is superfluous check.
> > > 
> > > Now in the form of this series, you have only a single dev_dbg() that tries to
> > > dereference it. Do we really need to have it there, since every branch has its
> > > own dev_dbg() anyway?
> > 
> > As I mentioned, I like to keep this check to show the reader that we
> > should only descend into gpiod_find_by_fwnode() if we have a valid
> > fwnode. It is less about code generation and more about the intent.
> 
> Yes, but if fwnode is not found, we have a next check for that.

No, the check you are talking about is for the GPIO not being located.
It does not have anything to do with fwnode validity. You are relying on
intimate knowledge of gpiod_find_by_fwnode() implementation and the fact
that in the current form it will withstand ERR_PTR-encoded or NULL
fwnode.

I want to have the source code so clear in its intent so that I can be
woken up in the middle of the night with a huge hangover and still be
able to tell how it is supposed to behave.

> I really don't
> think we lose anything by dropping the check and gaining the code generation as
> a side effect.

This is cold path, happening only on startup. I am not saying that we
want to make it slow unnecessarily, but a condition branch that might
even get optimized out is not something we should be concerned here.

Thanks.
  
Andy Shevchenko Nov. 10, 2022, 8:10 p.m. UTC | #5
On Thu, Nov 10, 2022 at 09:21:59AM -0800, Dmitry Torokhov wrote:
> On Thu, Nov 10, 2022 at 03:42:40PM +0200, Andy Shevchenko wrote:
> > On Wed, Nov 09, 2022 at 11:00:29AM -0800, Dmitry Torokhov wrote:
> > > On Wed, Nov 09, 2022 at 01:25:06PM +0200, Andy Shevchenko wrote:
> > > > On Tue, Nov 08, 2022 at 04:26:50PM -0800, Dmitry Torokhov wrote:

...

> > > > > +	if (!IS_ERR_OR_NULL(fwnode))
> > > > 
> > > > I think this is superfluous check.
> > > > 
> > > > Now in the form of this series, you have only a single dev_dbg() that tries to
> > > > dereference it. Do we really need to have it there, since every branch has its
> > > > own dev_dbg() anyway?
> > > 
> > > As I mentioned, I like to keep this check to show the reader that we
> > > should only descend into gpiod_find_by_fwnode() if we have a valid
> > > fwnode. It is less about code generation and more about the intent.
> > 
> > Yes, but if fwnode is not found, we have a next check for that.
> 
> No, the check you are talking about is for the GPIO not being located.
> It does not have anything to do with fwnode validity. You are relying on
> intimate knowledge of gpiod_find_by_fwnode() implementation and the fact
> that in the current form it will withstand ERR_PTR-encoded or NULL
> fwnode.
> 
> I want to have the source code so clear in its intent so that I can be
> woken up in the middle of the night with a huge hangover and still be
> able to tell how it is supposed to behave.

As you said let's leave it to Bart and Linus.

> > I really don't
> > think we lose anything by dropping the check and gaining the code generation as
> > a side effect.
> 
> This is cold path, happening only on startup. I am not saying that we
> want to make it slow unnecessarily, but a condition branch that might
> even get optimized out is not something we should be concerned here.

Agree, that's why I called it "side effect".
  

Patch

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 1bc386032ca8..bed0380c5136 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -1024,45 +1024,6 @@  struct gpio_desc *acpi_find_gpio(struct fwnode_handle *fwnode,
 	return desc;
 }
 
-/**
- * acpi_node_get_gpiod() - get a GPIO descriptor from ACPI resources
- * @fwnode: pointer to an ACPI firmware node to get the GPIO information from
- * @propname: Property name of the GPIO
- * @index: index of GpioIo/GpioInt resource (starting from %0)
- * @lflags: bitmask of gpio_lookup_flags GPIO_* values
- * @dflags: gpiod initialization flags
- *
- * If @fwnode is an ACPI device object, call acpi_get_gpiod_by_index() for it.
- * Otherwise (i.e. it is a data-only non-device object), use the property-based
- * GPIO lookup to get to the GPIO resource with the relevant information and use
- * that to obtain the GPIO descriptor to return.
- *
- * If the GPIO cannot be translated or there is an error an ERR_PTR is
- * returned.
- */
-struct gpio_desc *acpi_node_get_gpiod(struct fwnode_handle *fwnode,
-				      const char *propname, int index,
-				      unsigned long *lflags,
-				      enum gpiod_flags *dflags)
-{
-	struct acpi_gpio_info info;
-	struct acpi_device *adev;
-	struct gpio_desc *desc;
-
-	adev = to_acpi_device_node(fwnode);
-	if (adev)
-		desc = acpi_get_gpiod_by_index(adev, propname, index, &info);
-	else
-		desc = acpi_get_gpiod_from_data(fwnode, propname, index, &info);
-
-	if (!IS_ERR(desc)) {
-		acpi_gpio_update_gpiod_flags(dflags, &info);
-		acpi_gpio_update_gpiod_lookup_flags(lflags, &info);
-	}
-
-	return desc;
-}
-
 /**
  * acpi_dev_gpio_irq_wake_get_by() - Find GpioInt and translate it to Linux IRQ number
  * @adev: pointer to a ACPI device to get IRQ from
diff --git a/drivers/gpio/gpiolib-acpi.h b/drivers/gpio/gpiolib-acpi.h
index 8880615327ac..9475f99a9694 100644
--- a/drivers/gpio/gpiolib-acpi.h
+++ b/drivers/gpio/gpiolib-acpi.h
@@ -36,10 +36,6 @@  struct gpio_desc *acpi_find_gpio(struct fwnode_handle *fwnode,
 				 unsigned int idx,
 				 enum gpiod_flags *dflags,
 				 unsigned long *lookupflags);
-struct gpio_desc *acpi_node_get_gpiod(struct fwnode_handle *fwnode,
-				      const char *propname, int index,
-				      unsigned long *lflags,
-				      enum gpiod_flags *dflags);
 
 int acpi_gpio_count(struct device *dev, const char *con_id);
 #else
@@ -61,12 +57,6 @@  acpi_find_gpio(struct fwnode_handle *fwnode, const char *con_id,
 {
 	return ERR_PTR(-ENOENT);
 }
-static inline struct gpio_desc *
-acpi_node_get_gpiod(struct fwnode_handle *fwnode, const char *propname,
-		    int index, unsigned long *lflags, enum gpiod_flags *dflags)
-{
-	return ERR_PTR(-ENXIO);
-}
 static inline int acpi_gpio_count(struct device *dev, const char *con_id)
 {
 	return -ENODEV;
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index f0a7a59ac630..c33ffdd47667 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -3801,58 +3801,89 @@  static int platform_gpio_count(struct device *dev, const char *con_id)
 	return count;
 }
 
-/**
- * fwnode_get_named_gpiod - obtain a GPIO from firmware node
- * @fwnode:	handle of the firmware node
- * @propname:	name of the firmware property representing the GPIO
- * @index:	index of the GPIO to obtain for the consumer
- * @dflags:	GPIO initialization flags
- * @label:	label to attach to the requested GPIO
- *
- * This function can be used for drivers that get their configuration
- * from opaque firmware.
- *
- * The function properly finds the corresponding GPIO using whatever is the
- * underlying firmware interface and then makes sure that the GPIO
- * descriptor is requested before it is returned to the caller.
- *
- * Returns:
- * On successful request the GPIO pin is configured in accordance with
- * provided @dflags.
- *
- * In case of error an ERR_PTR() is returned.
- */
-static struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
-						const char *propname, int index,
-						enum gpiod_flags dflags,
-						const char *label)
+static struct gpio_desc *gpiod_find_by_fwnode(struct fwnode_handle *fwnode,
+					      struct device *consumer,
+					      const char *con_id,
+					      unsigned int idx,
+					      enum gpiod_flags *flags,
+					      unsigned long *lookupflags)
 {
-	unsigned long lflags = GPIO_LOOKUP_FLAGS_DEFAULT;
-	struct gpio_desc *desc = ERR_PTR(-ENODEV);
-	int ret;
+	struct gpio_desc *desc;
 
+	dev_dbg(consumer, "GPIO lookup for consumer %s in node '%pfw'\n",
+		con_id, fwnode);
+
+	/* Using device tree? */
 	if (is_of_node(fwnode)) {
-		desc = gpiod_get_from_of_node(to_of_node(fwnode),
-					      propname, index,
-					      dflags,
-					      label);
-		return desc;
+		dev_dbg(consumer, "using device tree for GPIO lookup\n");
+		desc = of_find_gpio(to_of_node(fwnode),
+				    con_id, idx, lookupflags);
 	} else if (is_acpi_node(fwnode)) {
-		desc = acpi_node_get_gpiod(fwnode, propname, index,
-					   &lflags, &dflags);
-		if (IS_ERR(desc))
-			return desc;
+		dev_dbg(consumer, "using ACPI for GPIO lookup\n");
+		desc = acpi_find_gpio(fwnode, con_id, idx, flags, lookupflags);
 	} else {
-		return ERR_PTR(-EINVAL);
+		desc = ERR_PTR(-ENOENT);
 	}
 
-	/* Currently only ACPI takes this path */
+	return desc;
+}
+
+static struct gpio_desc *gpiod_find_and_request(struct device *consumer,
+						struct fwnode_handle *fwnode,
+						const char *con_id,
+						unsigned int idx,
+						enum gpiod_flags flags,
+						const char *label,
+						bool platform_lookup_allowed)
+{
+	struct gpio_desc *desc = ERR_PTR(-ENOENT);
+	unsigned long lookupflags;
+	int ret;
+
+	if (!IS_ERR_OR_NULL(fwnode))
+		desc = gpiod_find_by_fwnode(fwnode, consumer, con_id, idx,
+					    &flags, &lookupflags);
+
+	if (gpiod_not_found(desc) && platform_lookup_allowed) {
+		/*
+		 * Either we are not using DT or ACPI, or their lookup did not
+		 * return a result. In that case, use platform lookup as a
+		 * fallback.
+		 */
+		dev_dbg(consumer, "using lookup tables for GPIO lookup\n");
+		desc = gpiod_find(consumer, con_id, idx, &lookupflags);
+	}
+
+	if (IS_ERR(desc)) {
+		dev_dbg(consumer, "No GPIO consumer %s found\n", con_id);
+		return desc;
+	}
+
+	/*
+	 * If a connection label was passed use that, else attempt to use
+	 * the device name as label
+	 */
 	ret = gpiod_request(desc, label);
-	if (ret)
-		return ERR_PTR(ret);
+	if (ret) {
+		if (!(ret == -EBUSY && flags & GPIOD_FLAGS_BIT_NONEXCLUSIVE))
+			return ERR_PTR(ret);
+
+		/*
+		 * This happens when there are several consumers for
+		 * the same GPIO line: we just return here without
+		 * further initialization. It is a bit of a hack.
+		 * This is necessary to support fixed regulators.
+		 *
+		 * FIXME: Make this more sane and safe.
+		 */
+		dev_info(consumer,
+			 "nonexclusive access to GPIO for %s\n", con_id);
+		return desc;
+	}
 
-	ret = gpiod_configure_flags(desc, propname, lflags, dflags);
+	ret = gpiod_configure_flags(desc, con_id, lookupflags, flags);
 	if (ret < 0) {
+		dev_dbg(consumer, "setup of GPIO %s failed\n", con_id);
 		gpiod_put(desc);
 		return ERR_PTR(ret);
 	}
@@ -3885,29 +3916,13 @@  static struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
  * In case of error an ERR_PTR() is returned.
  */
 struct gpio_desc *fwnode_gpiod_get_index(struct fwnode_handle *fwnode,
-					 const char *con_id, int index,
+					 const char *con_id,
+					 int index,
 					 enum gpiod_flags flags,
 					 const char *label)
 {
-	struct gpio_desc *desc;
-	char prop_name[32]; /* 32 is max size of property name */
-	unsigned int i;
-
-	for (i = 0; i < ARRAY_SIZE(gpio_suffixes); i++) {
-		if (con_id)
-			snprintf(prop_name, sizeof(prop_name), "%s-%s",
-					    con_id, gpio_suffixes[i]);
-		else
-			snprintf(prop_name, sizeof(prop_name), "%s",
-					    gpio_suffixes[i]);
-
-		desc = fwnode_get_named_gpiod(fwnode, prop_name, index, flags,
-					      label);
-		if (!gpiod_not_found(desc))
-			break;
-	}
-
-	return desc;
+	return gpiod_find_and_request(NULL, fwnode, con_id, index, flags, label,
+				      false);
 }
 EXPORT_SYMBOL_GPL(fwnode_gpiod_get_index);
 
@@ -4061,72 +4076,12 @@  struct gpio_desc *__must_check gpiod_get_index(struct device *dev,
 					       unsigned int idx,
 					       enum gpiod_flags flags)
 {
-	unsigned long lookupflags = GPIO_LOOKUP_FLAGS_DEFAULT;
-	struct gpio_desc *desc = NULL;
-	int ret;
-	/* Maybe we have a device name, maybe not */
-	const char *devname = dev ? dev_name(dev) : "?";
 	struct fwnode_handle *fwnode = dev ? dev_fwnode(dev) : NULL;
+	const char *devname = dev ? dev_name(dev) : "?";
+	const char *label = con_id ?: devname;
 
-	dev_dbg(dev, "GPIO lookup for consumer %s\n", con_id);
-
-	/* Using device tree? */
-	if (is_of_node(fwnode)) {
-		dev_dbg(dev, "using device tree for GPIO lookup\n");
-		desc = of_find_gpio(to_of_node(fwnode),
-				    con_id, idx, &lookupflags);
-	} else if (is_acpi_node(fwnode)) {
-		dev_dbg(dev, "using ACPI for GPIO lookup\n");
-		desc = acpi_find_gpio(fwnode,
-				      con_id, idx, &flags, &lookupflags);
-	}
-
-	/*
-	 * Either we are not using DT or ACPI, or their lookup did not return
-	 * a result. In that case, use platform lookup as a fallback.
-	 */
-	if (!desc || gpiod_not_found(desc)) {
-		dev_dbg(dev, "using lookup tables for GPIO lookup\n");
-		desc = gpiod_find(dev, con_id, idx, &lookupflags);
-	}
-
-	if (IS_ERR(desc)) {
-		dev_dbg(dev, "No GPIO consumer %s found\n", con_id);
-		return desc;
-	}
-
-	/*
-	 * If a connection label was passed use that, else attempt to use
-	 * the device name as label
-	 */
-	ret = gpiod_request(desc, con_id ?: devname);
-	if (ret) {
-		if (!(ret == -EBUSY && flags & GPIOD_FLAGS_BIT_NONEXCLUSIVE))
-			return ERR_PTR(ret);
-
-		/*
-		 * This happens when there are several consumers for
-		 * the same GPIO line: we just return here without
-		 * further initialization. It is a bit of a hack.
-		 * This is necessary to support fixed regulators.
-		 *
-		 * FIXME: Make this more sane and safe.
-		 */
-		dev_info(dev, "nonexclusive access to GPIO for %s\n", con_id ?: devname);
-		return desc;
-	}
-
-	ret = gpiod_configure_flags(desc, con_id, lookupflags, flags);
-	if (ret < 0) {
-		dev_dbg(dev, "setup of GPIO %s failed\n", con_id);
-		gpiod_put(desc);
-		return ERR_PTR(ret);
-	}
-
-	blocking_notifier_call_chain(&desc->gdev->notifier,
-				     GPIOLINE_CHANGED_REQUESTED, desc);
-
-	return desc;
+	return gpiod_find_and_request(dev, fwnode, con_id, idx, flags, label,
+				      true);
 }
 EXPORT_SYMBOL_GPL(gpiod_get_index);