On 07/12/2017 06:35 AM, Linus Walleij wrote:
On Tue, Jul 11, 2017 at 7:55 PM, Davide Hug <d@xxxxxxxxxx> wrote:I'm looking at a driver (hwmon/sht15) that does not load with message: gpio-48 (SHT15 data): _gpiod_direction_output_raw: tried to set a GPIO tied to an IRQ as output This happens because the driver uses a GPIO line alternately (and not at the same time) as IRQ and output. To do this irq_enable/disable are used to prevent setting a GPIO with enabled IRQ as output. This seems the right approach to me since in the patch "gpio: add API to be strict about GPIO IRQ usage"[1] I read: The API is symmetric so that lines can also be flagged from .irq_enable() and unflagged from IRQ by .irq_disable(). But I didn't find any GPIO driver implementing this API in irq_enable/disable. Since my platform uses omap_gpio which does not implement irq_enable/disable at all, I added a default irq_enable/disable implementation in gpio/gpiolib.c (in the patch below). Is it the right approach to call gpiochip_lock_as_irq/gpiochip_unlock_as_irq
It might solve your problem, but this is not the way to move forward :( Looks like, hwmon/sht15 is very specific driver ;) and, honestly, I think it better to use polling instead of IRQs.
We already call this in gpiochip_irq_reqres() and gpiochip_irq_relres(). from the .irq_request_resources()/.irq_release_resources() callbacks. Your patch makes us call the functions twice I think. Why isn't gpiochip_irq_relres() called before you reuse the line for something else than IRQ? If there is a legitimate reason why .irq_disable() and .irq_enable() are called but not .irq_request_resources() and .irq_release_resources() I want to know why that is so. TGLX implemented these functions in the first place exactly to be used for gpiochip_lock_as_irq()/gpiochip_unlock_as_irq(), see: commit c1bacbae8192dd2a9ebadd22d793b68054f6c6e5 "genirq: Provide irq_request/release_resources chip callbacks" Then we switch in commit 57ef04288abd27a717287a652d324f95cb77c3c6 "gpio: switch drivers to use new callback" At the time we were calling this from .irq_startup() and .irq_shutdown() and I am afraid that .irq_enable() and .irq_disable() will just recreate the same problem we had in 2014.
And they are always called from atomic context -- regards, -grygorii -- 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
