diff mbox

[v2,1/5] gpiolib: Allow GPIO chips to request their own GPIOs

Message ID 1394456094-21671-2-git-send-email-mika.westerberg@linux.intel.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Mika Westerberg March 10, 2014, 12:54 p.m. UTC
Sometimes it is useful to allow GPIO chips themselves to request GPIOs they
own through gpiolib API. One use case is ACPI ASL code that should be able
to toggle GPIOs through GPIO operation regions.

We can't use gpio_request() because it will pin the module to the kernel
forever (it calls try_module_get()). To solve this we move module refcount
manipulation to gpiod_request() and let __gpiod_request() handle the actual
request. This changes the sequence a bit as now try_module_get() is called
outside of gpio_lock (I think this is safe, try_module_get() handles
serialization it needs already).

Then we provide gpiolib internal functions gpiochip_request/free_own_desc()
that do the same as gpio_request() but don't manipulate module refrence
count. This allows the GPIO chip driver to request and free descriptors it
owns without being pinned to the kernel forever.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/gpio/gpiolib.c | 100 ++++++++++++++++++++++++++++++++++++-------------
 drivers/gpio/gpiolib.h |   3 ++
 2 files changed, 76 insertions(+), 27 deletions(-)

Comments

Alexandre Courbot March 13, 2014, 4:46 a.m. UTC | #1
On Mon, Mar 10, 2014 at 9:54 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> Sometimes it is useful to allow GPIO chips themselves to request GPIOs they
> own through gpiolib API. One use case is ACPI ASL code that should be able
> to toggle GPIOs through GPIO operation regions.
>
> We can't use gpio_request() because it will pin the module to the kernel
> forever (it calls try_module_get()). To solve this we move module refcount
> manipulation to gpiod_request() and let __gpiod_request() handle the actual
> request. This changes the sequence a bit as now try_module_get() is called
> outside of gpio_lock (I think this is safe, try_module_get() handles
> serialization it needs already).
>
> Then we provide gpiolib internal functions gpiochip_request/free_own_desc()
> that do the same as gpio_request() but don't manipulate module refrence
> count. This allows the GPIO chip driver to request and free descriptors it
> owns without being pinned to the kernel forever.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>

The change is clear and does not add too much complexity to the code,
so no reason to oppose it.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index f60d74bc2fce..61de1efe85fd 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1458,26 +1458,14 @@  EXPORT_SYMBOL_GPL(gpiochip_remove_pin_ranges);
  * on each other, and help provide better diagnostics in debugfs.
  * They're called even less than the "set direction" calls.
  */
-static int gpiod_request(struct gpio_desc *desc, const char *label)
+static int __gpiod_request(struct gpio_desc *desc, const char *label)
 {
-	struct gpio_chip	*chip;
-	int			status = -EPROBE_DEFER;
+	struct gpio_chip	*chip = desc->chip;
+	int			status;
 	unsigned long		flags;
 
-	if (!desc) {
-		pr_warn("%s: invalid GPIO\n", __func__);
-		return -EINVAL;
-	}
-
 	spin_lock_irqsave(&gpio_lock, flags);
 
-	chip = desc->chip;
-	if (chip == NULL)
-		goto done;
-
-	if (!try_module_get(chip->owner))
-		goto done;
-
 	/* NOTE:  gpio_request() can be called in early boot,
 	 * before IRQs are enabled, for non-sleeping (SOC) GPIOs.
 	 */
@@ -1487,7 +1475,6 @@  static int gpiod_request(struct gpio_desc *desc, const char *label)
 		status = 0;
 	} else {
 		status = -EBUSY;
-		module_put(chip->owner);
 		goto done;
 	}
 
@@ -1499,7 +1486,6 @@  static int gpiod_request(struct gpio_desc *desc, const char *label)
 
 		if (status < 0) {
 			desc_set_label(desc, NULL);
-			module_put(chip->owner);
 			clear_bit(FLAG_REQUESTED, &desc->flags);
 			goto done;
 		}
@@ -1511,9 +1497,34 @@  static int gpiod_request(struct gpio_desc *desc, const char *label)
 		spin_lock_irqsave(&gpio_lock, flags);
 	}
 done:
+	spin_unlock_irqrestore(&gpio_lock, flags);
+	return status;
+}
+
+static int gpiod_request(struct gpio_desc *desc, const char *label)
+{
+	int status = -EPROBE_DEFER;
+	struct gpio_chip *chip;
+
+	if (!desc) {
+		pr_warn("%s: invalid GPIO\n", __func__);
+		return -EINVAL;
+	}
+
+	chip = desc->chip;
+	if (!chip)
+		goto done;
+
+	if (try_module_get(chip->owner)) {
+		status = __gpiod_request(desc, label);
+		if (status < 0)
+			module_put(chip->owner);
+	}
+
+done:
 	if (status)
 		gpiod_dbg(desc, "%s: status %d\n", __func__, status);
-	spin_unlock_irqrestore(&gpio_lock, flags);
+
 	return status;
 }
 
@@ -1523,18 +1534,14 @@  int gpio_request(unsigned gpio, const char *label)
 }
 EXPORT_SYMBOL_GPL(gpio_request);
 
-static void gpiod_free(struct gpio_desc *desc)
+static bool __gpiod_free(struct gpio_desc *desc)
 {
+	bool			ret = false;
 	unsigned long		flags;
 	struct gpio_chip	*chip;
 
 	might_sleep();
 
-	if (!desc) {
-		WARN_ON(extra_checks);
-		return;
-	}
-
 	gpiod_unexport(desc);
 
 	spin_lock_irqsave(&gpio_lock, flags);
@@ -1548,15 +1555,23 @@  static void gpiod_free(struct gpio_desc *desc)
 			spin_lock_irqsave(&gpio_lock, flags);
 		}
 		desc_set_label(desc, NULL);
-		module_put(desc->chip->owner);
 		clear_bit(FLAG_ACTIVE_LOW, &desc->flags);
 		clear_bit(FLAG_REQUESTED, &desc->flags);
 		clear_bit(FLAG_OPEN_DRAIN, &desc->flags);
 		clear_bit(FLAG_OPEN_SOURCE, &desc->flags);
-	} else
-		WARN_ON(extra_checks);
+		ret = true;
+	}
 
 	spin_unlock_irqrestore(&gpio_lock, flags);
+	return ret;
+}
+
+static void gpiod_free(struct gpio_desc *desc)
+{
+	if (desc && __gpiod_free(desc))
+		module_put(desc->chip->owner);
+	else
+		WARN_ON(extra_checks);
 }
 
 void gpio_free(unsigned gpio)
@@ -1678,6 +1693,37 @@  const char *gpiochip_is_requested(struct gpio_chip *chip, unsigned offset)
 }
 EXPORT_SYMBOL_GPL(gpiochip_is_requested);
 
+/**
+ * gpiochip_request_own_desc - Allow GPIO chip to request its own descriptor
+ * @desc: GPIO descriptor to request
+ * @label: label for the GPIO
+ *
+ * Function allows GPIO chip drivers to request and use their own GPIO
+ * descriptors via gpiolib API. Difference to gpiod_request() is that this
+ * function will not increase reference count of the GPIO chip module. This
+ * allows the GPIO chip module to be unloaded as needed (we assume that the
+ * GPIO chip driver handles freeing the GPIOs it has requested).
+ */
+int gpiochip_request_own_desc(struct gpio_desc *desc, const char *label)
+{
+	if (!desc || !desc->chip)
+		return -EINVAL;
+
+	return __gpiod_request(desc, label);
+}
+
+/**
+ * gpiochip_free_own_desc - Free GPIO requested by the chip driver
+ * @desc: GPIO descriptor to free
+ *
+ * Function frees the given GPIO requested previously with
+ * gpiochip_request_own_desc().
+ */
+void gpiochip_free_own_desc(struct gpio_desc *desc)
+{
+	if (desc)
+		__gpiod_free(desc);
+}
 
 /* Drivers MUST set GPIO direction before making get/set calls.  In
  * some cases this is done in early boot, before IRQs are enabled.
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 82be586c1f90..cf092941a9fd 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -43,4 +43,7 @@  acpi_get_gpiod_by_index(struct device *dev, int index,
 }
 #endif
 
+int gpiochip_request_own_desc(struct gpio_desc *desc, const char *label);
+void gpiochip_free_own_desc(struct gpio_desc *desc);
+
 #endif /* GPIOLIB_H */