On Mon, Sep 27, 2010 at 10:16:11AM +0200, Par-Gunnar Hjalmdahl wrote: > 2010/9/24 Mark Brown <broonie@xxxxxxxxxxxxxxxxxxxxxxxxxxx>: > > On Fri, Sep 24, 2010 at 03:46:44PM +0200, Par-Gunnar Hjalmdahl wrote: > >> +static irqreturn_t cg2900_devices_interrupt(int irq, void *dev_id) > >> +{ > >> + disable_irq_nosync(irq); > >> + if (cg2900_dev_callback && cg2900_dev_callback->interrupt_cb) > >> + cg2900_dev_callback->interrupt_cb(); > >> + > >> + return IRQ_HANDLED; > >> +} > > Why is there this callback mechanism - I'd expect the users of this code > > to just be using the standard IRQ infrastructure? > This is our local IRQ which is handled in cg2900_uart.c by creating a work: I can tell it's an IRQ - that's my point; the kernel has an interrupt handling subsystem which this isn't using. > I understand your concern that client implementing the > cg2900_dev_callback->interrupt_cb(); > might not know realized that this is irq context which might cause problems. No, you're misunderstanding my point. The kernel has facilities for representing interrupt controllers which this is not using. > If not then suggest what is expected way of handling it ? eg. moving > workqueue down to cg2900_devices > or other solution? Use the standard kernel IRQ framework to report interrupts to the clients - there's quite a few examples of using this in the drivers/mfd code. This will make your code more idiomatic and give you access to all the diagnostic infrastructure the kernel provides for interrupts. -- To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html