From patchwork Fri Jul 6 11:25:49 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans Verkuil X-Patchwork-Id: 940424 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=linux-gpio-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=xs4all.nl Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 41MXTs09Fsz9s4Z for ; Fri, 6 Jul 2018 21:25:53 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932265AbeGFLZw (ORCPT ); Fri, 6 Jul 2018 07:25:52 -0400 Received: from lb1-smtp-cloud9.xs4all.net ([194.109.24.22]:57292 "EHLO lb1-smtp-cloud9.xs4all.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932174AbeGFLZv (ORCPT ); Fri, 6 Jul 2018 07:25:51 -0400 Received: from [IPv6:2001:983:e9a7:1:c0df:c193:2d8f:16db] ([IPv6:2001:983:e9a7:1:c0df:c193:2d8f:16db]) by smtp-cloud9.xs4all.net with ESMTPA id bOs1fsmh0EJtcbOs2fEmFN; Fri, 06 Jul 2018 13:25:50 +0200 To: linux-gpio@vger.kernel.org From: Hans Verkuil Subject: [RFC PATCH] gpiolib: call gpiochip_(un)lock_as_irq for dis/enable_irq() Message-ID: <8c586fd0-7deb-82bf-4c3b-3e2d0a3d1738@xs4all.nl> Date: Fri, 6 Jul 2018 13:25:49 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 Content-Language: en-US X-CMAE-Envelope: MS4wfH9XsvfqwvatSoJ2W9TgECk52gGAzDPJdbKwgn+mpvkQ4rK0xnjJyz+m0eLI8jISPkKnPjrmtxibI56paShnIAUCMgNaVK9Ad3W24ARthEcgjpBUgxqe HKi4r/+G+jE9ATqGd52fZwBUhkv8+T2JyKwjB15rvJO8OZZABnfyuvmFc7ktFp2265bx3tSVz8cdw6RJ1yGfsTZrilDhQZUz+Gem8nEZfG6Jo+s+OBvmwtp2 esWnT6Y5DY+mCgk9wGThgu5AteFanXH0QnVavMw4cws= Sender: linux-gpio-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-gpio@vger.kernel.org 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 Signed-off-by: Hans Verkuil --- -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html 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);