On Tue, May 27, 2014 at 11:01:03AM +0200, Hans de Goede wrote: > Hi, > > On 05/27/2014 10:09 AM, Maxime Ripard wrote: > > On Mon, May 26, 2014 at 09:47:58AM +0200, Hans de Goede wrote: > >> With level triggered interrupt mask / unmask will get called for each > >> interrupt, doing the somewhat expensive mux setting on each unmask thus is > >> not a good idea. Instead move it to the set_type callback, which is typically > >> done only once for each irq. > > > > *This* is the bad idea. Nothing prevents you from calling > > gpio_get_value whenever you just got your interrupt, that will change > > the muxing, and never change it back, effectively breaking the > > interrupts. > > Hmm, interesting point, but your assuming 2 things which are both not true: > > 1) That calling gpiod_get_value changes the muxing, which is not true, all > gpiod_get_value variants end up in drivers/gpio/gpiolib.c _gpiod_get_raw_value() > which does no such thing Somehow I was convinced it was the case, but yeah, it doesn't really make much sense, especially since you can get the value of an output, without wanting to change it to input. > > 2) That unmask will always get called after the gpio_get_value to restore the mux > setting for us, which is not guaranteed at all. Before this patch set pinctrl-sunxi.c > was using handle_simple_irq which does not mask / unmask at all. > > And even with an irq-handler which masks / unmasks, what if the irq wakes up > a thread and that thread then does the gpio_get_value ? > > Note this is *exactly* what e.g. the mmc gpio card-detect code does, it uses > a thread to read the gpio for debouncing. > > Luckily 2. is not a problem, since 1. means that the mux won't get changed at all > so we don't need to change it back. Ok. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
Attachment:
signature.asc
Description: Digital signature
