Moving staging:iio over to threaded interrupts.

Moving staging:iio over to threaded interrupts.

[Jonathan Cameron]

Hi All,

Given the timing of the writing of most of the IIO core was 
prior to threaded interrupts hitting the mainstream we didn't
use them originally. I suspect that the large numbers of old style
interrupts that we have is going to be a bit of a blocker when we
move out of staging so have been thinking about what to do about it.

This email is intended to get some initial thoughts from others
(some of whom are hopefully more familiar with the ins and outs
of threaded interrupts than I am!)

As I understand it, the primary point of threaded interrupts is
to replace bottom half handlers (conventionally done on using
work queues) with kernel threads that are woken on the interrupt.
The reason is that kernel threads can be pre empted whereas work
queue items can not.

We have two common types of interrupt in IIO.

* Event interrupts (threshold detectors etc). I think these should
  be trivial to convert to threaded irq's.  For those that need
  timestamps we will still need a top half handler to grab that
  but it should be easy enough to do.  There are nasty cases where
  a given physical line can do both this and the next case.  With
  care we can probably avoid this being an issue by careful switching
  of the interrupt handlers according to what can occur in a given
  configuration of the device.

* Trigger interrupts - These are more complex.

For those not that familiar with the IIO trigger infrastructure let
me run through its intent and give an outline of how it currently works.

Our current triggers fall into two classes:
1) Those associated with a device - data ready signals typically
2) Those independent of a device - gpio, timer, userspace.

For simplicity we treat both of these classes the same.

Any trigger can have a number of devices registered with it.  This
allows for, as near a possible, simultaneous reading from a set of
devices.  Which devices these are can be configured / changed at
runtime.

There are two activation lists and each device driver instance can register
in one or both of these. The first 'fast' queue is intended for devices that
have a 'latch' or 'capture' signal typically consisting of a gpio
linked pin.  (no in tree users, but hardware with this
is common enough). Basically the intent is this list launches true
top half elements.

The second 'slow' queue currently queues up work queue items to read
from each of the devices.  It's this part that would ideally be threaded.

Currently the infrastructure is nice and clean and more or less looks
the same as drivers doing their own handling of the relevant interrupts.

The problem lies in the fact each actual interrupt can effectively
call multiple bottom halves.

So to my mind two solutions exist.
1) A single thread per trigger.  Everything prior to the work queue
calls is moved into a handler that goes in the 'fast' list which stays
in our top half handler.  The work queue bits are called one after
another in the bottom half.

2) Allow each consumer to attach it's own thread to the trigger
controller and basically implement our own variant of the core threaded
interrupt code that allows for a list of threads rather than a single one.

I rather like the idea of 2.  It might even end up with different
devices being queried from different processor cores simultaneously
which is quite cute.  The question is whether a simple enough
implementation is possible that the originators of the threaded interrupt
code would be happy with it (as it bypasses or would mean additions to their
core code).

I originally sort opinions from IIO developers on this, but as no
one has been forthcoming with opinions am reposting to a wider
audience to get suggestions on how to handle this.

Thanks and please cc anyone who might want to offer opinions on this.

Thanks,

Jonathan

Re: Moving staging:iio over to threaded interrupts.

[Thomas Gleixner]

On Thu, 3 Mar 2011, Jonathan Cameron wrote:
> So to my mind two solutions exist.
> 1) A single thread per trigger.  Everything prior to the work queue
> calls is moved into a handler that goes in the 'fast' list which stays
> in our top half handler.  The work queue bits are called one after
> another in the bottom half.
> 
> 2) Allow each consumer to attach it's own thread to the trigger
> controller and basically implement our own variant of the core threaded
> interrupt code that allows for a list of threads rather than a single one.
> 
> I rather like the idea of 2.  It might even end up with different
> devices being queried from different processor cores simultaneously
> which is quite cute.  The question is whether a simple enough
> implementation is possible that the originators of the threaded interrupt
> code would be happy with it (as it bypasses or would mean additions to their
> core code).

Don't implement another threading model. Look at the trigger irq as a
demultiplexing interrupt. So if you have several consumers of a single
trigger, then you can implement a pseudo irq_chip and register the sub
devices as separate interrupts.

That means your main trigger interrupt would look like this:

irqreturn_t hardirq_handler(int irq, void *dev)
{
     iio_trigger_dev *idev = dev;
     int i;

     store_state_as_necessary(idev);

     for (i = 0; i < idev->nr_subirqs; i++) {
     	    if (idev->subirqs[i].enabled)
		generic_handle_irq(idev->subirq_base + i);
     }
}

And you'd have an irq_chip implementation which does:

static void subirq_mask(struct irq_data *d)
{
     iio_trigger_dev *idev = irq_data_get_irq_chip_data(d);
     int idx = d->irq - idev->subirq_base;

     idev->subirqs[idx].enabled = false;
}

static void subirq_unmask(struct irq_data *d)
{
     iio_trigger_dev *idev = irq_data_get_irq_chip_data(d);
     int idx = d->irq - idev->subirq_base;

     idev->subirqs[idx].enabled = true;
}

static struct irq_chip subirq_chip = {
       .name = "iiochip",
       .mask = subirq_mask,
       .unmask = subirq_unmask,
};

init()
{
	for_each_subirq(i)
		irq_set_chip_and_handler(i, &subirq_chip, handle_simple_irq);
}

So now you can request the interrupts for your subdevices with
request_irq or request_threaded_irq.

You can also implement #1 this way, you just mark the sub device
interrupts as IRQ_NESTED_THREAD, and then call the handlers from the
main trigger irq thread.

Thanks,

	tglx

Re: Moving staging:iio over to threaded interrupts.

[Jonathan Cameron]

On 03/08/11 10:30, Thomas Gleixner wrote:
> On Thu, 3 Mar 2011, Jonathan Cameron wrote:
>> So to my mind two solutions exist.
>> 1) A single thread per trigger.  Everything prior to the work queue
>> calls is moved into a handler that goes in the 'fast' list which stays
>> in our top half handler.  The work queue bits are called one after
>> another in the bottom half.
>>
>> 2) Allow each consumer to attach it's own thread to the trigger
>> controller and basically implement our own variant of the core threaded
>> interrupt code that allows for a list of threads rather than a single one.
>>
>> I rather like the idea of 2.  It might even end up with different
>> devices being queried from different processor cores simultaneously
>> which is quite cute.  The question is whether a simple enough
>> implementation is possible that the originators of the threaded interrupt
>> code would be happy with it (as it bypasses or would mean additions to their
>> core code).
> 
> Don't implement another threading model. Look at the trigger irq as a
> demultiplexing interrupt. So if you have several consumers of a single
> trigger, then you can implement a pseudo irq_chip and register the sub
> devices as separate interrupts.
> 
> That means your main trigger interrupt would look like this:
> 
> irqreturn_t hardirq_handler(int irq, void *dev)
> {
>      iio_trigger_dev *idev = dev;
>      int i;
> 
>      store_state_as_necessary(idev);
> 
>      for (i = 0; i < idev->nr_subirqs; i++) {
>      	    if (idev->subirqs[i].enabled)
> 		generic_handle_irq(idev->subirq_base + i);
>      }
> }
> 
> And you'd have an irq_chip implementation which does:
> 
> static void subirq_mask(struct irq_data *d)
> {
>      iio_trigger_dev *idev = irq_data_get_irq_chip_data(d);
>      int idx = d->irq - idev->subirq_base;
> 
>      idev->subirqs[idx].enabled = false;
> }
> 
> static void subirq_unmask(struct irq_data *d)
> {
>      iio_trigger_dev *idev = irq_data_get_irq_chip_data(d);
>      int idx = d->irq - idev->subirq_base;
> 
>      idev->subirqs[idx].enabled = true;
> }
> 
> static struct irq_chip subirq_chip = {
>        .name = "iiochip",
>        .mask = subirq_mask,
>        .unmask = subirq_unmask,
> };
> 
> init()
> {
> 	for_each_subirq(i)
> 		irq_set_chip_and_handler(i, &subirq_chip, handle_simple_irq);
> }
> 
> So now you can request the interrupts for your subdevices with
> request_irq or request_threaded_irq.
> 
> You can also implement #1 this way, you just mark the sub device
> interrupts as IRQ_NESTED_THREAD, and then call the handlers from the
> main trigger irq thread.
Excellent.  I hadn't thought of doing it that way at all and it certainly looks
like a much cleaner option than what we have now let alone the mess I was suggesting
above.

Will have a go at implementing this asap.

Thanks for the advice,

Jonathan

Re: Moving staging:iio over to threaded interrupts.

[Jonathan Cameron]

On 03/08/11 10:30, Thomas Gleixner wrote:
> On Thu, 3 Mar 2011, Jonathan Cameron wrote:
>> So to my mind two solutions exist.
>> 1) A single thread per trigger.  Everything prior to the work queue
>> calls is moved into a handler that goes in the 'fast' list which stays
>> in our top half handler.  The work queue bits are called one after
>> another in the bottom half.
>>
>> 2) Allow each consumer to attach it's own thread to the trigger
>> controller and basically implement our own variant of the core threaded
>> interrupt code that allows for a list of threads rather than a single one.
>>
>> I rather like the idea of 2.  It might even end up with different
>> devices being queried from different processor cores simultaneously
>> which is quite cute.  The question is whether a simple enough
>> implementation is possible that the originators of the threaded interrupt
>> code would be happy with it (as it bypasses or would mean additions to their
>> core code).
> 
> Don't implement another threading model. Look at the trigger irq as a
> demultiplexing interrupt. So if you have several consumers of a single
> trigger, then you can implement a pseudo irq_chip and register the sub
> devices as separate interrupts.
> 
> That means your main trigger interrupt would look like this:
> 
> irqreturn_t hardirq_handler(int irq, void *dev)
> {
>      iio_trigger_dev *idev = dev;
>      int i;
> 
>      store_state_as_necessary(idev);
> 
>      for (i = 0; i < idev->nr_subirqs; i++) {
>      	    if (idev->subirqs[i].enabled)
> 		generic_handle_irq(idev->subirq_base + i);
>      }
> }
> 
> And you'd have an irq_chip implementation which does:
> 
> static void subirq_mask(struct irq_data *d)
> {
>      iio_trigger_dev *idev = irq_data_get_irq_chip_data(d);
>      int idx = d->irq - idev->subirq_base;
> 
>      idev->subirqs[idx].enabled = false;
> }
> 
> static void subirq_unmask(struct irq_data *d)
> {
>      iio_trigger_dev *idev = irq_data_get_irq_chip_data(d);
>      int idx = d->irq - idev->subirq_base;
> 
>      idev->subirqs[idx].enabled = true;
> }
> 
> static struct irq_chip subirq_chip = {
>        .name = "iiochip",
>        .mask = subirq_mask,
>        .unmask = subirq_unmask,
> };
> 
> init()
> {
> 	for_each_subirq(i)
> 		irq_set_chip_and_handler(i, &subirq_chip, handle_simple_irq);
> }
> 
> So now you can request the interrupts for your subdevices with
> request_irq or request_threaded_irq.
> 
> You can also implement #1 this way, you just mark the sub device
> interrupts as IRQ_NESTED_THREAD, and then call the handlers from the
> main trigger irq thread.
Hi Thomas,

One issue here that I'm not quite sure how to overcome is that the trigger to
device mapping tends to be dynamic.  That is we quite often switch around
what device is triggered by which trigger at runtime.  All done via text label
matching via sysfs.

I guess we could maintain this by a spot of indirection and pool of interrupts per
trigger (with compile time control on how many). Any other approaches come to mind?

Thanks,

Jonathan

Re: Moving staging:iio over to threaded interrupts.

[Thomas Gleixner]

On Tue, 8 Mar 2011, Jonathan Cameron wrote:
> On 03/08/11 10:30, Thomas Gleixner wrote:
> > So now you can request the interrupts for your subdevices with
> > request_irq or request_threaded_irq.
> > 
> > You can also implement #1 this way, you just mark the sub device
> > interrupts as IRQ_NESTED_THREAD, and then call the handlers from the
> > main trigger irq thread.
> Hi Thomas,
> 
> One issue here that I'm not quite sure how to overcome is that the trigger to
> device mapping tends to be dynamic.  That is we quite often switch around
> what device is triggered by which trigger at runtime.  All done via text label
> matching via sysfs.

Was not aware of that.
 
> I guess we could maintain this by a spot of indirection and pool of interrupts per
> trigger (with compile time control on how many). Any other approaches come to mind?

That should work. You just need a function in the trigger
implementation which hands back an unused irq number to the device
when a trigger is installed for a device. Then the device calls
request[_threaded]_irq() on that irq number and all should work
magically.

Thanks,

	tglx

Re: Moving staging:iio over to threaded interrupts.

[Jonathan Cameron]

On 03/08/11 12:12, Thomas Gleixner wrote:
> On Tue, 8 Mar 2011, Jonathan Cameron wrote:
>> On 03/08/11 10:30, Thomas Gleixner wrote:
>>> So now you can request the interrupts for your subdevices with
>>> request_irq or request_threaded_irq.
>>>
>>> You can also implement #1 this way, you just mark the sub device
>>> interrupts as IRQ_NESTED_THREAD, and then call the handlers from the
>>> main trigger irq thread.
>> Hi Thomas,
>>
>> One issue here that I'm not quite sure how to overcome is that the trigger to
>> device mapping tends to be dynamic.  That is we quite often switch around
>> what device is triggered by which trigger at runtime.  All done via text label
>> matching via sysfs.
> 
> Was not aware of that.
>  
>> I guess we could maintain this by a spot of indirection and pool of interrupts per
>> trigger (with compile time control on how many). Any other approaches come to mind?
> 
> That should work. You just need a function in the trigger
> implementation which hands back an unused irq number to the device
> when a trigger is installed for a device. Then the device calls
> request[_threaded]_irq() on that irq number and all should work
> magically.
> 
Cool.  Actually thinking more on this we probably want to have a single pool for IIO
in general that then allocates sets to the individual drivers.  That way things
like dynamic trigger creation (which is needed for the bridge to the input subsystem
but not currently implemented) become possible.

Thanks for the quick reply!

Jonathan

Re: Moving staging:iio over to threaded interrupts.

[Jonathan Cameron]

On 03/08/11 12:26, Jonathan Cameron wrote:
> On 03/08/11 12:12, Thomas Gleixner wrote:
>> On Tue, 8 Mar 2011, Jonathan Cameron wrote:
>>> On 03/08/11 10:30, Thomas Gleixner wrote:
>>>> So now you can request the interrupts for your subdevices with
>>>> request_irq or request_threaded_irq.
>>>>
>>>> You can also implement #1 this way, you just mark the sub device
>>>> interrupts as IRQ_NESTED_THREAD, and then call the handlers from the
>>>> main trigger irq thread.
>>> Hi Thomas,
>>>
>>> One issue here that I'm not quite sure how to overcome is that the trigger to
>>> device mapping tends to be dynamic.  That is we quite often switch around
>>> what device is triggered by which trigger at runtime.  All done via text label
>>> matching via sysfs.
>>
>> Was not aware of that.
>>  
>>> I guess we could maintain this by a spot of indirection and pool of interrupts per
>>> trigger (with compile time control on how many). Any other approaches come to mind?
>>
>> That should work. You just need a function in the trigger
>> implementation which hands back an unused irq number to the device
>> when a trigger is installed for a device. Then the device calls
>> request[_threaded]_irq() on that irq number and all should work
>> magically.
>>
> Cool.  Actually thinking more on this we probably want to have a single pool for IIO
> in general that then allocates sets to the individual drivers.  That way things
> like dynamic trigger creation (which is needed for the bridge to the input subsystem
> but not currently implemented) become possible.
> 
Hi Thomas,

One question that I gather comes up from time to time but google isn't furnishing me
with an answer. I'd imagine it something to do with having to know where interrupts should
go early in boot, but best ask anyway.

I'm getting undefined errors for irq_to_desc, set_irq_chip_and_handler, handle_simple_irq
and set_irq_flags.

Why are irq_to_desc etc not usable from a module? I really would rather avoid building
the core of IIO in. I suppose I could move this corner out on it's own so that it alone
is built in.

I'm guessing the approach I have right now were my trigger allocate function (which can easily
be called from a module) creates and connects up the irq_chip on demand is a very bad idea?

Is the right way to do this to handle all my 'virtual' interrupts as a single chip in some built
in function then make their handler play some games underneath?

Thanks again for your help.

Too many years of just assuming this underlying interrupt stuff worked ;)

Jonathan