Messages in this thread | | | Date | Mon, 9 Jun 2008 09:57:01 +0200 | From | Uwe Kleine-König <> | Subject | Re: [PATCH] uio_pdrv: Unique IRQ Mode |
| |
Hello Hans,
> Did you notice that in this thread nobody spoke up to support your > patch? Actually I like what the patch tries to achieve. I'd like to have it a bit more explicit tough:
- Provide the irq disabling handler in uio_pdrv.c (or even uio.c) with a prototype in an adequate header. Then the platforms that want this kind of handling can request it explicitly.
- Don't use this handler automatically.
- Provide the function named uio_pdrv_unique_irqcontrol in Magnus' patch in uio_pdrv.c and in an adequate header.
- Either rely on userspace to enable the irq before reading/polling or assert that in kernel space. See also http://thread.gmane.org/gmane.linux.kernel/684683/focus=689635 (I asked tglx about the race condition via irc, but without a response so far.) Currently the former is done, but if we decide to let it as it is, I'd like to have it documented. (I.e. something like: "Before polling/reading /dev/uioX assert that irqs are enabled.")
The last point is a bit independent from that mode, but applies to devices that have a irqcontrol function in general.
Apart from the general things above, I'd change a few things in the implementation:
- call dev_info->irqcontrol(OFF) in the handler (instead of disable_irq()) and demand that calling this is idempotent. With this change it isn't uio_pdrv specific any more and could go to uio.c.
- rename "Unique IRQ Mode" to something like "No IRQ Handler Mode". I'm not completely lucky with that name, but it's better than the former. Of course the function should be renamed accordingly.
- s/unsigned long irq_disabled/unsigned int irq_disabled : 1/ in struct uio_platdata.
Best regards Uwe
-- Uwe Kleine-König, Software Engineer Digi International GmbH Branch Breisach, Küferstrasse 8, 79206 Breisach, Germany Tax: 315/5781/0242 / VAT: DE153662976 / Reg. Amtsgericht Dortmund HRB 13962 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |