Re: [PATCH] uio_pdrv: Unique IRQ Mode

["Uwe Kleine-König" <Uwe.Kleine-Koenig@digi.com>]

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