Re: [PATCH 7/8 v3] i2c: i801: enable irq for i801 smbustransactions

[Jean Delvare]

Hi Daniel,

On Fri, 6 Jul 2012 18:28:28 +0800, Daniel Kurtz wrote:
> On Thu, Jul 5, 2012 at 4:10 PM, Jean Delvare <khali@xxxxxxxxxxxx> wrote:
> > On Thu, 5 Jul 2012 12:31:11 +0800, Daniel Kurtz wrote:
> >> The logic for clearing the STATUS_FLAGS was simply that they all
> >> indicate the end of a transaction, so there won't be any further
> >> interrupts.  Thus, it is safe to clear it in the irq, since the irq
> >> will be followed by exactly one wait_event, which can read and process
> >> saved status.
> >
> > It is safe if and only if someone is actually listening to the event.
> > This was not the case for me yesterday. Even if we don't mix interrupts
> > and polling, i801_isr() might still get called when we aren't
> > listening. Not only because the IRQ may be shared, but also for events
> > we don't yet handle, such as an SMBus Alert. Not sure about slave
> > mode.
> 
> The real reason for clearing the flag in the hard irq is that
> otherwise, we end up with an infinite irq loop.  The interrupt is
> apparently level triggered, and must be cleared before exiting the
> ISR.

Oh, OK. If we have to do it that way, then we do.

> Unfortunately, I only had time today to discover this, but not time to fix it.
> Next week I will be away, so I won't have a chance to provide more
> patches for the next 10 days.
> 
> I think the 6 patches you have already make a complete set, however,
> and shouldn't be waiting for the interrupt patch(es) which can follow
> at some future date.

Agreed, that's why they are already in linux-next waiting for the next
merge window to open.

That being said, I would hate to miss that merge window for interrupt
support patches, as this is really a great performance improvement. So,
if you have no objection, I propose to fix your patches myself today of
tomorrow, test them, and if I find no problem, post them and push them
to linux-next. You can review and test them when you return.

-- 
Jean Delvare
--
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/