[RFCv2 PATCH 1/4] gpiolib: (un)mark gpio as irq when dis/enabling irq

[Hans Verkuil <hverkuil@xxxxxxxxx>]

From: Hans Verkuil <hans.verkuil@xxxxxxxxx>

Drivers are currently required to call gpiochip_(un)lock_as_irq whenever
they want to use a gpio as an interrupt. Unfortunately, this is generally
done when the irq is requested, not when the irq is enabled/disabled.

This is problematic for cases where a gpio pin is temporarily used as an
interrupt pin, then, after the irq is disabled, is used as a regular
gpio pin again. Currently it is not possible to do this other than by first
freeing the interrupt so gpiochip_unlock_as_irq is called.

There are currently two drivers that would like to be able to do this:
the tda998x_drv.c driver where a regular gpio pin needs to be temporarily
reconfigured as an interrupt pin during CEC calibration, and the cec-gpio
driver where you want to configure the gpio pin as an interrupt while
waiting for traffic over the CEC bus, or as a regular pin when receiving or
transmitting a CEC message.

The core problem is that gpiochip_(un)lock_as_irq is called when the
interrupt is allocated/freed, not when the interrupt is enabled/disabled.

Also, it is hit-and-miss whether drivers call these functions.

This patch hooks gpiochip_irq_(un)lock into the irq_enable/disable
callbacks of the irqchip. That way drivers no longer have to call these
functions and this is all done transparently for drivers.

The old gpiochip_(un)lock_as_irq() functions are now empty stubs that
can be removed once all drivers that call them are updated.

Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
---
 drivers/gpio/gpiolib.c      | 155 +++++++++++++++++++++++++++---------
 include/linux/gpio/driver.h |   7 ++
 2 files changed, 123 insertions(+), 39 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e8f8a1999393..0efa9ec4fec4 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -86,6 +86,8 @@ static int gpiochip_add_irqchip(struct gpio_chip *gpiochip,
 static void gpiochip_irqchip_remove(struct gpio_chip *gpiochip);
 static int gpiochip_irqchip_init_valid_mask(struct gpio_chip *gpiochip);
 static void gpiochip_irqchip_free_valid_mask(struct gpio_chip *gpiochip);
+static int gpiochip_irq_lock(struct gpio_chip *chip, unsigned int offset);
+static void gpiochip_irq_unlock(struct gpio_chip *chip, unsigned int offset);
 
 static bool gpiolib_initialized;
 
@@ -1807,30 +1809,65 @@ static const struct irq_domain_ops gpiochip_domain_ops = {
 static int gpiochip_irq_reqres(struct irq_data *d)
 {
 	struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
-	int ret;
+	int ret = 0;
 
 	if (!try_module_get(chip->gpiodev->owner))
 		return -ENODEV;
 
-	ret = gpiochip_lock_as_irq(chip, d->hwirq);
-	if (ret) {
-		chip_err(chip,
-			"unable to lock HW IRQ %lu for IRQ\n",
-			d->hwirq);
+	if (chip->irq.irq_request_resources)
+		ret = chip->irq.irq_request_resources(d);
+	if (ret)
 		module_put(chip->gpiodev->owner);
-		return ret;
-	}
-	return 0;
+	return ret;
 }
 
 static void gpiochip_irq_relres(struct irq_data *d)
 {
 	struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
 
-	gpiochip_unlock_as_irq(chip, d->hwirq);
+	if (chip->irq.irq_release_resources)
+		chip->irq.irq_release_resources(d);
 	module_put(chip->gpiodev->owner);
 }
 
+static unsigned int gpiochip_irq_startup(struct irq_data *d)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
+
+	WARN_ON(gpiochip_irq_lock(chip, d->hwirq));
+	return chip->irq.irq_startup(d);
+}
+
+static void gpiochip_irq_shutdown(struct irq_data *d)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
+
+	gpiochip_irq_unlock(chip, d->hwirq);
+	chip->irq.irq_shutdown(d);
+}
+
+static void gpiochip_irq_enable(struct irq_data *d)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
+
+	WARN_ON(gpiochip_irq_lock(chip, d->hwirq));
+	if (chip->irq.irq_enable)
+		chip->irq.irq_enable(d);
+	else
+		chip->irq.chip->irq_unmask(d);
+}
+
+static void gpiochip_irq_disable(struct irq_data *d)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
+
+	gpiochip_irq_unlock(chip, d->hwirq);
+	if (chip->irq.irq_disable)
+		chip->irq.irq_disable(d);
+	else
+		chip->irq.chip->irq_mask(d);
+}
+
 static int gpiochip_to_irq(struct gpio_chip *chip, unsigned offset)
 {
 	if (!gpiochip_irqchip_irq_valid(chip, offset))
@@ -1839,6 +1876,33 @@ static int gpiochip_to_irq(struct gpio_chip *chip, unsigned offset)
 	return irq_create_mapping(chip->irq.domain, offset);
 }
 
+static void gpiochip_set_irq_hooks(struct gpio_chip *gpiochip)
+{
+	struct irq_chip *irqchip = gpiochip->irq.chip;
+
+	if (irqchip->irq_request_resources == gpiochip_irq_reqres)
+		return;
+
+	gpiochip->irq.irq_request_resources = irqchip->irq_request_resources;
+	gpiochip->irq.irq_release_resources = irqchip->irq_release_resources;
+	gpiochip->irq.irq_enable = irqchip->irq_enable;
+	gpiochip->irq.irq_disable = irqchip->irq_disable;
+
+	irqchip->irq_request_resources = gpiochip_irq_reqres;
+	irqchip->irq_release_resources = gpiochip_irq_relres;
+	irqchip->irq_enable = gpiochip_irq_enable;
+	irqchip->irq_disable = gpiochip_irq_disable;
+
+	if (irqchip->irq_startup) {
+		gpiochip->irq.irq_startup = irqchip->irq_startup;
+		irqchip->irq_startup = gpiochip_irq_startup;
+	}
+	if (irqchip->irq_shutdown) {
+		gpiochip->irq.irq_shutdown = irqchip->irq_shutdown;
+		irqchip->irq_shutdown = gpiochip_irq_shutdown;
+	}
+}
+
 /**
  * gpiochip_add_irqchip() - adds an IRQ chip to a GPIO chip
  * @gpiochip: the GPIO chip to add the IRQ chip to
@@ -1897,16 +1961,6 @@ static int gpiochip_add_irqchip(struct gpio_chip *gpiochip,
 	if (!gpiochip->irq.domain)
 		return -EINVAL;
 
-	/*
-	 * It is possible for a driver to override this, but only if the
-	 * alternative functions are both implemented.
-	 */
-	if (!irqchip->irq_request_resources &&
-	    !irqchip->irq_release_resources) {
-		irqchip->irq_request_resources = gpiochip_irq_reqres;
-		irqchip->irq_release_resources = gpiochip_irq_relres;
-	}
-
 	if (gpiochip->irq.parent_handler) {
 		void *data = gpiochip->irq.parent_handler_data ?: gpiochip;
 
@@ -1922,6 +1976,8 @@ static int gpiochip_add_irqchip(struct gpio_chip *gpiochip,
 		}
 	}
 
+	gpiochip_set_irq_hooks(gpiochip);
+
 	acpi_gpiochip_request_interrupts(gpiochip);
 
 	return 0;
@@ -1935,11 +1991,12 @@ static int gpiochip_add_irqchip(struct gpio_chip *gpiochip,
  */
 static void gpiochip_irqchip_remove(struct gpio_chip *gpiochip)
 {
+	struct irq_chip *irqchip = gpiochip->irq.chip;
 	unsigned int offset;
 
 	acpi_gpiochip_free_interrupts(gpiochip);
 
-	if (gpiochip->irq.chip && gpiochip->irq.parent_handler) {
+	if (irqchip && gpiochip->irq.parent_handler) {
 		struct gpio_irq_chip *irq = &gpiochip->irq;
 		unsigned int i;
 
@@ -1963,11 +2020,26 @@ static void gpiochip_irqchip_remove(struct gpio_chip *gpiochip)
 		irq_domain_remove(gpiochip->irq.domain);
 	}
 
-	if (gpiochip->irq.chip) {
-		gpiochip->irq.chip->irq_request_resources = NULL;
-		gpiochip->irq.chip->irq_release_resources = NULL;
-		gpiochip->irq.chip = NULL;
+	if (irqchip &&
+	    irqchip->irq_request_resources == gpiochip_irq_reqres) {
+		irqchip->irq_request_resources =
+			gpiochip->irq.irq_request_resources;
+		irqchip->irq_release_resources =
+			gpiochip->irq.irq_release_resources;
+		irqchip->irq_enable = gpiochip->irq.irq_enable;
+		irqchip->irq_disable = gpiochip->irq.irq_disable;
+		if (gpiochip->irq.irq_startup)
+			irqchip->irq_startup = gpiochip->irq.irq_startup;
+		if (gpiochip->irq.irq_shutdown)
+			irqchip->irq_shutdown = gpiochip->irq.irq_shutdown;
 	}
+	gpiochip->irq.irq_request_resources = NULL;
+	gpiochip->irq.irq_release_resources = NULL;
+	gpiochip->irq.irq_startup = NULL;
+	gpiochip->irq.irq_shutdown = NULL;
+	gpiochip->irq.irq_enable = NULL;
+	gpiochip->irq.irq_disable = NULL;
+	gpiochip->irq.chip = NULL;
 
 	gpiochip_irqchip_free_valid_mask(gpiochip);
 }
@@ -2056,15 +2128,7 @@ int gpiochip_irqchip_add_key(struct gpio_chip *gpiochip,
 		return -EINVAL;
 	}
 
-	/*
-	 * It is possible for a driver to override this, but only if the
-	 * alternative functions are both implemented.
-	 */
-	if (!irqchip->irq_request_resources &&
-	    !irqchip->irq_release_resources) {
-		irqchip->irq_request_resources = gpiochip_irq_reqres;
-		irqchip->irq_release_resources = gpiochip_irq_relres;
-	}
+	gpiochip_set_irq_hooks(gpiochip);
 
 	acpi_gpiochip_request_interrupts(gpiochip);
 
@@ -3255,14 +3319,14 @@ int gpiod_to_irq(const struct gpio_desc *desc)
 EXPORT_SYMBOL_GPL(gpiod_to_irq);
 
 /**
- * gpiochip_lock_as_irq() - lock a GPIO to be used as IRQ
+ * gpiochip_irq_lock() - lock a GPIO to be used as IRQ
  * @chip: the chip the GPIO to lock belongs to
  * @offset: the offset of the GPIO to lock as IRQ
  *
  * This is used directly by GPIO drivers that want to lock down
  * a certain GPIO line to be used for IRQs.
  */
-int gpiochip_lock_as_irq(struct gpio_chip *chip, unsigned int offset)
+static int gpiochip_irq_lock(struct gpio_chip *chip, unsigned int offset)
 {
 	struct gpio_desc *desc;
 
@@ -3303,17 +3367,16 @@ int gpiochip_lock_as_irq(struct gpio_chip *chip, unsigned int offset)
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(gpiochip_lock_as_irq);
 
 /**
- * gpiochip_unlock_as_irq() - unlock a GPIO used as IRQ
+ * gpiochip_irq_unlock() - unlock a GPIO used as IRQ
  * @chip: the chip the GPIO to lock belongs to
  * @offset: the offset of the GPIO to lock as IRQ
  *
  * This is used directly by GPIO drivers that want to indicate
  * that a certain GPIO is no longer used exclusively for IRQ.
  */
-void gpiochip_unlock_as_irq(struct gpio_chip *chip, unsigned int offset)
+static void gpiochip_irq_unlock(struct gpio_chip *chip, unsigned int offset)
 {
 	struct gpio_desc *desc;
 
@@ -3327,6 +3390,20 @@ void gpiochip_unlock_as_irq(struct gpio_chip *chip, unsigned int offset)
 	if (desc->label && !strcmp(desc->label, "interrupt"))
 		desc_set_label(desc, NULL);
 }
+
+/*
+ * Keep these two temporary stubs until all drivers stop calling these
+ * functions.
+ */
+int gpiochip_lock_as_irq(struct gpio_chip *chip, unsigned int offset)
+{
+	return 0;
+}
+EXPORT_SYMBOL_GPL(gpiochip_lock_as_irq);
+
+void gpiochip_unlock_as_irq(struct gpio_chip *chip, unsigned int offset)
+{
+}
 EXPORT_SYMBOL_GPL(gpiochip_unlock_as_irq);
 
 bool gpiochip_line_is_irq(struct gpio_chip *chip, unsigned int offset)
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 0ea328e71ec9..0485bd339178 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -138,6 +138,13 @@ struct gpio_irq_chip {
 	 * will allocate and map all IRQs during initialization.
 	 */
 	unsigned int first;
+
+	int		(*irq_request_resources)(struct irq_data *data);
+	void		(*irq_release_resources)(struct irq_data *data);
+	unsigned int	(*irq_startup)(struct irq_data *data);
+	void		(*irq_shutdown)(struct irq_data *data);
+	void		(*irq_enable)(struct irq_data *data);
+	void		(*irq_disable)(struct irq_data *data);
 };
 
 static inline struct gpio_irq_chip *to_gpio_irq_chip(struct irq_chip *chip)
-- 
2.18.0