Messages in this thread | | | Date | Wed, 4 Jun 2008 12:11:45 +0200 | From | "Hans J. Koch" <> | Subject | Re: [PATCH] uio_pdrv: Unique IRQ Mode |
| |
On Wed, Jun 04, 2008 at 03:08:26PM +0900, Magnus Damm wrote: > From: Magnus Damm <damm@igel.co.jp> > > This patch adds a "Unique IRQ Mode" to the uio_pdrv UIO platform driver. > In this mode the user space driver is responsible for acknowledging and > re-enabling the interrupt. Shared interrupts are not supported.
I still don't see any gain in this. This only works for embedded devices, so a user has to setup hardware specific code in his board support anyway. With your code, we would have to add something like this to the docs:
IF you define an irq AND ommit the irq handler THEN we silently add a handler that blindly assumes the irq is not shared...
In my opinion, this is confusing, and all it does is saving the need for a three-lines irq handler in the board support.
So, NAK to this until somebody convinces me that I completely missed the point.
Thanks, Hans
> > Signed-off-by: Magnus Damm <damm@igel.co.jp> > --- > > Think of this as V2 of "[PATCH 00/03][RFC] Reusable UIO Platform Driver". > Needs "[PATCH 0/1] UIO: Add a write() function to enable/disable interrupts" > > drivers/uio/uio_pdrv.c | 43 +++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 43 insertions(+) > > --- 0006/drivers/uio/uio_pdrv.c > +++ work/drivers/uio/uio_pdrv.c 2008-06-04 14:51:56.000000000 +0900 > @@ -16,8 +16,34 @@ > > struct uio_platdata { > struct uio_info *uioinfo; > + unsigned long irq_disabled; > }; > > +static irqreturn_t uio_pdrv_unique_handler(int irq, struct uio_info *dev_info) > +{ > + struct uio_platdata *priv = dev_info->priv; > + > + /* In "Unique IRQ Mode", just disable the interrupt and remember > + * the state so we can enable it later. > + */ > + disable_irq(irq); > + set_bit(0, &priv->irq_disabled); > + return IRQ_HANDLED; > +} > + > +static int uio_pdrv_unique_irqcontrol(struct uio_info *dev_info, s32 irq_on) > +{ > + struct uio_platdata *priv = dev_info->priv; > + > + /* "Unique IRQ Mode" allows re-enabling of the interrupt */ > + if (irq_on && test_and_clear_bit(0, &priv->irq_disabled)) { > + enable_irq(dev_info->irq); > + return 0; > + } > + > + return -ENOSYS; > +} > + > static int uio_pdrv_probe(struct platform_device *pdev) > { > struct uio_info *uioinfo = pdev->dev.platform_data; > @@ -68,6 +94,23 @@ static int uio_pdrv_probe(struct platfor > > pdata->uioinfo->priv = pdata; > > + /* This driver supports a special "Unique IRQ Mode". > + * > + * In this mode, no hardware specific kernel code is required to > + * acknowledge interrupts. Instead, the interrupt is disabled by > + * the interrupt handler. User space is responsible for performing > + * hardware specific acknowledge and enabling of interrupts. > + * > + * Interrupt sharing is _not_ supported by the "Unique IRQ Mode". > + * > + * Enable this mode by passing IRQ number but omitting irq callbacks. > + */ > + if (!uioinfo->handler && !uioinfo->irqcontrol && uioinfo->irq >= 0) { > + uioinfo->irq_flags = IRQF_DISABLED; > + uioinfo->handler = uio_pdrv_unique_handler; > + uioinfo->irqcontrol = uio_pdrv_unique_irqcontrol; > + } > + > ret = uio_register_device(&pdev->dev, pdata->uioinfo); > > if (ret) {
| |