On Sat, 07 Nov 2015 17:32:55 +0100, Maarten Brock wrote:
> > On Fri, 6 Nov 2015 16:48:20 +0100, Maarten Brock wrote:
> > > Enable shared interrupts for the sc16is7xx serial driver.
> > >
> > > Signed-off-by: Maarten Brock <m.brock@xxxxxxxxxxxxx>
> > > ---
> > > drivers/tty/serial/sc16is7xx.c | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/tty/serial/sc16is7xx.c
> > b/drivers/tty/serial/sc16is7xx.c
> > > index 72ffd0d..13f5f69 100644
> > > --- a/drivers/tty/serial/sc16is7xx.c
> > > +++ b/drivers/tty/serial/sc16is7xx.c
> > > @@ -1230,7 +1230,8 @@ static int sc16is7xx_probe(struct device *dev,
> > >
> > > /* Setup interrupt */
> > > ret = devm_request_irq(dev, irq, sc16is7xx_irq,
> > > - IRQF_ONESHOT | flags, dev_name(dev), s);
> > > + IRQF_SHARED | IRQF_ONESHOT | flags,
> > > + dev_name(dev), s);
> > > if (!ret)
> > > return 0;
> > >
> >
> > The reason why shared IRQs were not enabled is that we cannot read
> > the status register from IRQ handler and therefore cannot decide if
> > the interrupt was actually ours (we always return IRQ_HANDLED). Would
> > you consider setting the IRQF_SHARED in your device tree instead of
> > changing the default?
>
> Ok, I think I understand why it's difficult or impossible to read the status
> register from the IRQ handler. But why does that pose a problem when you
> always return IRQ_HANDLED? Your last remark seems to indicate it doesn't.
I think it doesn't, you'll just loose the ability to detect spurious
interrupts. We never mask our IRQ so you should be fine (IIUC ONESHOT
flag is ignored for non-threaded IRQs).
> Or would you recommend never to use shared interrupts with these devices?
Sure, always try not to share IRQ line. Sharing IRQs wastes CPU cycles
and increases the interrupt latency. I see nothing wrong with your
patch per-se if it works for you, though. If you still want it
included in mainline you should probably repost putting Greg KH into
To: and Cc: linux-serial@xxxxxxxxxxxxxxx.
> Btw. I tried to set it through the device tree, but that didn't seem to work.
> sc16is740: sc16is740@0 {
> compatible = "nxp,sc16is740";
> <snip>
> interrupts = <25 0x82>; /* 0x80 for shared, 0x02 for falling */
> }
Sorry if it doesn't work, just something which came to my mind.
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
