diff mbox series

gpio: Add device link support

Message ID 20190621152329.21072-1-thierry.reding@gmail.com
State New
Headers show
Series gpio: Add device link support | expand

Commit Message

Thierry Reding June 21, 2019, 3:23 p.m. UTC 
From: Thierry Reding <treding@nvidia.com>

Create a device link between a GPIO consumer and the GPIO supplier (i.e.
the GPIO chip's parent). This makes the driver core aware of the
dependency between the two devices so that it can properly order the
suspend/resume sequence.

As a side-effect the GPIO consumer will be force unloaded when the GPIO
supplier goes away, which prevents the consumer from accessing dangling
GPIOs.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpio/gpiolib-devres.c |  4 ++++
 drivers/gpio/gpiolib.c        | 20 +++++++++++++++++++-
 drivers/gpio/gpiolib.h        |  2 ++
 3 files changed, 25 insertions(+), 1 deletion(-)

Comments

Linus Walleij June 24, 2019, 10:34 p.m. UTC | #1 
On Fri, Jun 21, 2019 at 5:23 PM Thierry Reding <thierry.reding@gmail.com> wrote:

> From: Thierry Reding <treding@nvidia.com>
>
> Create a device link between a GPIO consumer and the GPIO supplier (i.e.
> the GPIO chip's parent). This makes the driver core aware of the
> dependency between the two devices so that it can properly order the
> suspend/resume sequence.
>
> As a side-effect the GPIO consumer will be force unloaded when the GPIO
> supplier goes away, which prevents the consumer from accessing dangling
> GPIOs.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>

I was thinking we were gonna see this soon :D

> +void gpiod_add_device_link(struct gpio_desc *desc, struct device *consumer)
> +{
> +       struct device_link *link;
> +
> +       link = device_link_add(consumer, desc->gdev->dev.parent,
> +                              DL_FLAG_AUTOREMOVE_CONSUMER);

I think this needs a NULL check.

Some GPIO chips still have NULL as parent here, they just
register a gpio_chip right out of boardfile code without any
corresponding device. MIPS does this for example IIRC.

> +               device_link_remove(desc->consumer, desc->gdev->dev.parent);

And same on remove.

But why not use desc->gdev->dev as link, simply?
That device has the same lifetime as the gpio_chip, sometimes
even longer I think, if e.g. there is userspace holding the
GPIOs.

Yours,
Linus Walleij
diff mbox series

Patch

diff --git a/drivers/gpio/gpiolib-devres.c b/drivers/gpio/gpiolib-devres.c
index 0acc2cc6e868..0092feec9a5a 100644
--- a/drivers/gpio/gpiolib-devres.c
+++ b/drivers/gpio/gpiolib-devres.c
@@ -177,6 +177,8 @@  struct gpio_desc *devm_gpiod_get_from_of_node(struct device *dev,
 		return ERR_PTR(-ENOMEM);
 	}
 
+	gpiod_add_device_link(desc, dev);
+
 	*dr = desc;
 	devres_add(dev, dr);
 
@@ -234,6 +236,8 @@  struct gpio_desc *devm_fwnode_get_index_gpiod_from_child(struct device *dev,
 		return desc;
 	}
 
+	gpiod_add_device_link(desc, dev);
+
 	*dr = desc;
 	devres_add(dev, dr);
 
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 71cd685ed6c4..bd793038ba36 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4160,6 +4160,20 @@  int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id,
 	return status;
 }
 
+void gpiod_add_device_link(struct gpio_desc *desc, struct device *consumer)
+{
+	struct device_link *link;
+
+	link = device_link_add(consumer, desc->gdev->dev.parent,
+			       DL_FLAG_AUTOREMOVE_CONSUMER);
+	if (!link) {
+		dev_err(consumer, "failed to create device link to %s\n",
+			dev_name(desc->gdev->dev.parent));
+	} else {
+		desc->consumer = consumer;
+	}
+}
+
 /**
  * gpiod_get_index - obtain a GPIO from a multi-index GPIO function
  * @dev:	GPIO consumer, can be NULL for system-global GPIOs
@@ -4242,6 +4256,8 @@  struct gpio_desc *__must_check gpiod_get_index(struct device *dev,
 		return ERR_PTR(status);
 	}
 
+	gpiod_add_device_link(desc, dev);
+
 	return desc;
 }
 EXPORT_SYMBOL_GPL(gpiod_get_index);
@@ -4625,8 +4641,10 @@  EXPORT_SYMBOL_GPL(gpiod_get_array_optional);
  */
 void gpiod_put(struct gpio_desc *desc)
 {
-	if (desc)
+	if (desc) {
+		device_link_remove(desc->consumer, desc->gdev->dev.parent);
 		gpiod_free(desc);
+	}
 }
 EXPORT_SYMBOL_GPL(gpiod_put);
 
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 7a65dad43932..ed1488aedce0 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -215,6 +215,7 @@  extern struct list_head gpio_devices;
 
 struct gpio_desc {
 	struct gpio_device	*gdev;
+	struct device		*consumer;
 	unsigned long		flags;
 /* flag symbols are bit numbers */
 #define FLAG_REQUESTED	0
@@ -241,6 +242,7 @@  int gpiod_request(struct gpio_desc *desc, const char *label);
 void gpiod_free(struct gpio_desc *desc);
 int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id,
 		unsigned long lflags, enum gpiod_flags dflags);
+void gpiod_add_device_link(struct gpio_desc *desc, struct device *consumer);
 int gpiod_hog(struct gpio_desc *desc, const char *name,
 		unsigned long lflags, enum gpiod_flags dflags);