Re: [PATCH] uio_pdrv: Unique IRQ Mode

From: Hans J. Koch
Date: Fri Jun 06 2008 - 06:04:50 EST


On Fri, Jun 06, 2008 at 11:55:30AM +0900, Magnus Damm wrote:
> Hi Uwe,
>
> On Thu, Jun 5, 2008 at 3:49 PM, Uwe Kleine-KÃnig
> <Uwe.Kleine-Koenig@xxxxxxxx> wrote:
> > Hello Magnus,
> >
> > Magnus Damm wrote:
> >> ... "Unique IRQ Mode". ...
> > BTW, I wouldn't call it "Unique IRQ Mode" because the non-shared irq is
> > only a result from automatically disabling the irq. IMHO something like
> > "No IRQ Handler Mode" is more suitable.
>
> That may be a good idea. Any name is fine with me.
>
> Hopefully a better name will make people less confused. =)

I didn't criticize the name. My objection is that you want to introduce
an optional special handling for cases where the interrupt line is not
shared, e.g. a GPIO line. In that case, the handler _could_ look like
this:

static irqreturn_t my_handler(int irq, struct uio_info *info)
{
irq_disable(MY_GPIO_LINE);
return IRQ_HANDLED;
}

This solution is only second best, if possible, the irq should be
properly acknowledged within the chip, but it could be done like this.
Note that this doesn't work on every platform, it assumes that each GPIO
line has its own irq number. (Did you ever think about fixing Kconfig so
that this option is disabled on platforms where it is not possible or
not sensible to do this?)

You now suggest that if somebody doesn't fill in an irq handler, we
should make the above the default. This would save somebody the trouble
to add the above 5 lines to the 30 lines of board/platform support code
he has to write anyway. That's the only gain, and that is not enough.

Giving it a different name doesn't make it better.

Thanks,
Hans

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/