Commit fa38869b authored Aug 20, 2018 by Uwe Kleine-König Committed by Linus Walleij Sep 13, 2018

gpiolib: Don't support irq sharing for userspace



This concerns gpio edge detection for GPIO IRQs used from
userspace for GPIO event listeners.

Trying to work out the right event if it's not sure that the
examined gpio actually moved is impossible.

Consider two gpios "gpioA" and "gpioB" that share an interrupt.
gpioA's irq should trigger on any edge, gpioB's on a falling edge.
If now the common irq fires and both gpio lines are high, there
are several possibilities that could have happend:

 a) gpioA just had a low-to-high edge
 b) gpioB just had a high-to-low-to-high spike
 c) a combination of both a) and b)

While c) is unlikely (in most setups) a) and b) alone are bad
enough. Currently the code assumes case a) unconditionally and
doesn't report an event for gpioB. Note that even if there is no
irq sharing involved a spike for a gpio might not result in an
event if it's configured to trigger for a single edge only.

The only way to improve this is to drop support for interrupt
sharing. This way a spike results in an event for the right gpio
at least. Note that apart from dropping IRQF_SHARED this
effectively undoes commit df1e76f2
("gpiolib: skip unwanted events, don't convert them to opposite edge").

This obviously breaks setups that rely on interrupt sharing,
but given that this cannot be reliable, this is probably an
acceptable trade-off.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
[Assuming there are no users of interrupt sharing yet]
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

parent 0b35cd7b

drivers/gpio/gpiolib.c

+4 −5

Original line number	Diff line number	Diff line
		@@ -812,26 +812,26 @@ static irqreturn_t lineevent_irq_thread(int irq, void *p)
		{
		struct lineevent_state *le = p;
		struct gpioevent_data ge;
		int ret, level;
		int ret;

		/* Do not leak kernel stack to userspace */
		memset(&ge, 0, sizeof(ge));

		ge.timestamp = le->timestamp;
		level = gpiod_get_value_cansleep(le->desc);

		if (le->eflags & GPIOEVENT_REQUEST_RISING_EDGE
		&& le->eflags & GPIOEVENT_REQUEST_FALLING_EDGE) {
		int level = gpiod_get_value_cansleep(le->desc);
		if (level)
		/* Emit low-to-high event */
		ge.id = GPIOEVENT_EVENT_RISING_EDGE;
		else
		/* Emit high-to-low event */
		ge.id = GPIOEVENT_EVENT_FALLING_EDGE;
		} else if (le->eflags & GPIOEVENT_REQUEST_RISING_EDGE && level) {
		} else if (le->eflags & GPIOEVENT_REQUEST_RISING_EDGE) {
		/* Emit low-to-high event */
		ge.id = GPIOEVENT_EVENT_RISING_EDGE;
		} else if (le->eflags & GPIOEVENT_REQUEST_FALLING_EDGE && !level) {
		} else if (le->eflags & GPIOEVENT_REQUEST_FALLING_EDGE) {
		/* Emit high-to-low event */
		ge.id = GPIOEVENT_EVENT_FALLING_EDGE;
		} else {
		@@ -942,7 +942,6 @@ static int lineevent_create(struct gpio_device gdev, void __user ip)
		if (eflags & GPIOEVENT_REQUEST_FALLING_EDGE)
		irqflags \|= IRQF_TRIGGER_FALLING;
		irqflags \|= IRQF_ONESHOT;
		irqflags \|= IRQF_SHARED;

		INIT_KFIFO(le->events);
		init_waitqueue_head(&le->wait);

Admin message