Re: [PATCH][RESEND] i2c-i801: Add basic interrupt support

[Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>]

On Wed, 20 Aug 2008 06:53:48 +0200, Ivo Manca wrote:
> Hey Jean,
> 
> Jean Delvare wrote:
> >>> OK, I see what's going on. On this laptop, IRQ 9 is used by many
> >>> things, not just SMBus. So the interrupt handler keeps being called even
> >>> without SMBus activity or with polled-based SMBus activity. That's what
> >>> happens during SMBus block transactions: the interrupt handler is
> >>> called but not for us. However the interrupt handler thinks it is
> >>> called by us and clears the status register value. This causes the
> >>> polled-based loop to wait forever: by the time it looks for the status
> >>> register value, it has been cleared.
> >>>
> >>> So we need to change the code in either of three ways:
> >>> * Drop support for byte-by-byte block transactions.
> >>> * Inhibit the interrupt handler during polled-based block transactions.
> >>> * Convert the byteb-by-byte block transaction code to use interrupts
> >>>   instead of polling.
> >>> The latter would be cleaner, but that's also more work.
> >>>       
> I tried monday to add interrupt support to the byte-by-byte block 
> transactions, but it's not working yet. Keeps messing up (and eventually 
> crashing) my I2C bus.

I did some experiment too but didn't have too much success either. My
last attempt froze the system completely and I finally had to give up
due to a lack of time.

> >> Erm, is this testing with or without your merging of the status flags defines? 
> >> Maybe that is causing this?
> >
> > This is indeed with my merging of the status flag defines. But the
> > problem would happen regardless: the interrupt handler resets all the
> > status flags if any flag of I801_HST_STS_MASK_ALL, not just
> > I801_HST_STS_MASK_NORM, is set.
> >
> > You might argue that the interrupt handler could be modified to only
> > reset the flags it knows about, i.e. I801_HST_STS_MASK_NORM. This is
> > probably correct, but even then, it doesn't feel safe to have an
> > interrupt handler triggered while poll-based code operated on the same
> > register values
>
> True, it shouldn't happen. However, always only resetting flags it knows 
> about seems to make sense in every case, or not?

I will be fine with anything that works.

One important thing is that you must not clear the BYTE_DONE flag
before you are done handling the current data byte of the block
transaction. I guess that the interrupt handler should be modified to
read the block data byte and store it in the private data structure
(as it does with the status register) before clearing the flags.

> Not sure when i'll have some more time to look into it though...

Same problem here...

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c