[RFC PATCH] gpiolib: call gpiochip_(un)lock_as_irq for dis/enable_irq()

[Hans Verkuil <hverkuil@xxxxxxxxx>]

If a gpio pin has an interrupt associated with it, but you need to set the
direction of the pin to output, then what you want to do is to disable the
interrupt and change direction to output. And after changing the direction
back to input, then you call enable_irq again.

This does not work today since FLAG_USED_AS_IRQ is set when the interrupt
is first requested and gpiod_direction_output() checks for that. So the
only option is to free the interrupt and re-request it, which is painful.

This RFC patch sets hooks that allow the driver to know when the irq is
disabled and enabled and it sets FLAG_USED_AS_IRQ accordingly.

It works, but I have one open question:

When gpiochip_irq_enable() is called, I currently just WARN if
gpiochip_lock_as_irq() fails and continue after that. It really is
a driver bug, but I wonder if it is also possible to automatically
change the direction to input before enabling the interrupt. I have no
idea how to do this, though, since gpiod_direction_input() needs a
struct gpio_desc whereas in irq_enable() I have a struct gpio_chip.

This patch would be really useful for two CEC drivers:

In drivers/media/platform/cec-gpio/cec-gpio.c I have to free and
re-request the irq (cec_gpio_dis/enable_irq()). This driver implements
low-level support for the HDMI CEC bus. When waiting for something to
happen the gpio pin is set to input with an interrupt to avoid polling.
When writing to the bus it is set to output and the interrupt obviously
has to be removed (or, as I would like to see, just disabled). Requesting
an irq can fail, and handling that is really problematic. Just disabling
and enabling is much easier and cleaner.

The other driver is drivers/gpu/drm/i2c/tda998x_drv.c, specifically the
tda998x_cec_calibration() function: this is also a CEC driver but here
the tda998x interrupt pin is overloaded: when calibrating the CEC line
the interrupt pin of the chip is used as a gpio calibration output signal.

This driver disables the interrupt and switches the direction to output
before doing the calibration. This fails for the BeagleBone board unless
I apply this patch. This driver was originally developed for the dove-cubox
board which uses this gpio driver: drivers/gpio/gpio-mvebu.c. For some
reason this worked fine (i.e. it is apparently possible to disable the irq
and change the output without running into the FLAG_USED_AS_IRQ check), but
I don't understand why. I don't have this hardware, so I can't test it.

I'm a newbie when it comes to gpio, so I have no idea how much sense this
patch makes.

Regards,

	Hans

Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
---
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e11a3bb03820..dcdb457b83b0 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1822,6 +1822,20 @@ static void gpiochip_irq_relres(struct irq_data *d)
 	module_put(chip->gpiodev->owner);
 }

+static void gpiochip_irq_enable(struct irq_data *d)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
+
+	WARN_ON(gpiochip_lock_as_irq(chip, d->hwirq));
+}
+
+static void gpiochip_irq_disable(struct irq_data *d)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
+
+	gpiochip_unlock_as_irq(chip, d->hwirq);
+}
+
 static int gpiochip_to_irq(struct gpio_chip *chip, unsigned offset)
 {
 	if (!gpiochip_irqchip_irq_valid(chip, offset))
@@ -1897,6 +1911,10 @@ static int gpiochip_add_irqchip(struct gpio_chip *gpiochip,
 	    !irqchip->irq_release_resources) {
 		irqchip->irq_request_resources = gpiochip_irq_reqres;
 		irqchip->irq_release_resources = gpiochip_irq_relres;
+		if (!irqchip->irq_enable && !irqchip->irq_disable) {
+			irqchip->irq_enable = gpiochip_irq_enable;
+			irqchip->irq_disable = gpiochip_irq_disable;
+		}
 	}

 	if (gpiochip->irq.parent_handler) {
@@ -2056,6 +2074,10 @@ int gpiochip_irqchip_add_key(struct gpio_chip *gpiochip,
 	    !irqchip->irq_release_resources) {
 		irqchip->irq_request_resources = gpiochip_irq_reqres;
 		irqchip->irq_release_resources = gpiochip_irq_relres;
+		if (!irqchip->irq_enable && !irqchip->irq_disable) {
+			irqchip->irq_enable = gpiochip_irq_enable;
+			irqchip->irq_disable = gpiochip_irq_disable;
+		}
 	}

 	acpi_gpiochip_request_interrupts(gpiochip);
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html