On 12/11/15 13:12, Linus Walleij wrote: > It really doesn't matter if the poll is triggered from a > fastpath or threaded IRQ, the IIO core runs its own interrupt > thread anyway. Sometimes this is connected to a hard IRQ line, > sometimes to something on an I2C expander that needs to > run from a thread, so request any context IRQ. > > Cc: Denis Ciocca <denis.ciocca@xxxxxx> > Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx> Hi Linus, Note first that there is nothing stopping any random device hanging off the trigger - so we can't make any device specific assumptions. >From the start the way it was built up pretty much assumed we had a real hardware interrupt to drive this. Interesting question is does it matter - we jump through hoops to drop back into interrupt context at times. Perhaps we don't need to - I can't honestly remember why we did this and it was pre threaded interrupts so perhaps the assumptions are no longer valid. My knowledge of the irq handling is perhaps not deep enough but I can follow through code ;) So here goes. The result of this as you've identified is that we may call the function iio_trigger_generic_data_ready_poll from either a thread context or a interrupt context. This then calls generic_handle_irq for each of devices hanging off the trigger and ultimately desc->handle_irq and on to handle_simple_irq (fun this isn't it ;) This calls onto handle_irq_event and after a few more jumps ends up at handle_irq_event_percpu in kernel/irq/handle.c This calls the top half interrupt just fine an then if it gets an IRQ_WAKE_THREAD (most of the time) we call __irq_wake_thread. So the only remaining question is there anything that actually requires the thread to be woken from interrupt context... Ah, this is where it come unstuck... __irq_wake_thread explicitly makes the point that it it is running only on one cpu at a time and in interrupt context. Now, we could restrict the st_sensors trigger to use by the device supplying it - we could then operate the entire trigger always in a state where sleeping is safe (and run it as a chained irq - entirely in a thread). We could perhaps do some 'magic' to allow this on the irq side. As the interrupt is never threaded anyway (though the stuff hanging off it almost always is) we could do something nefarious such as: Call request_any_context_irq. If this returns IRQC_IS_HARDIRQ continue as normal. If it returns IRQ_C_IS_NESTED we could note this fact and handle any child interrupts hanging off the device differently. In the first instance we could refuse to let any pollfuncs with a top half interrupt bind to the trigger. The trigger would then call iio_trigger_poll_chained to call the nested interrupts (fine I think?) Am I missing something here? This turned out to be a rather deeper rabbit hole than I thought it would be when I started! Jonathan > --- > drivers/iio/common/st_sensors/st_sensors_trigger.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/iio/common/st_sensors/st_sensors_trigger.c b/drivers/iio/common/st_sensors/st_sensors_trigger.c > index 3c0aa17d753f..e33796cdd607 100644 > --- a/drivers/iio/common/st_sensors/st_sensors_trigger.c > +++ b/drivers/iio/common/st_sensors/st_sensors_trigger.c > @@ -32,9 +32,8 @@ int st_sensors_allocate_trigger(struct iio_dev *indio_dev, > goto iio_trigger_alloc_error; > } > > - err = request_threaded_irq(sdata->get_irq_data_ready(indio_dev), > + err = request_any_context_irq(sdata->get_irq_data_ready(indio_dev), > iio_trigger_generic_data_rdy_poll, > - NULL, > IRQF_TRIGGER_RISING, > sdata->trig->name, > sdata->trig); > -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
