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 -- 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