Re: [Qemu-devel] [PATCH] TSC2005 interrupt handling fix

[Riihimaki Juha (Nokia-D-MSW/Helsinki)]

Both issues are timing related; it is possible to get a timer tick  
callback in the TSC2005 while executing the guest ISR that is handling  
the interrupt raised by the TSC2005 earlier.

For issue #1 specifically, when this happens with the current version,  
all DAV bits for the running conversion function are set by the timer  
tick callback even if some of the results had already been read out  
and the corresponding DAV bits cleared. Now, when the guest ISR is  
finished and it clears the IRQ, some DAV bits in the TSC2005 will  
remain set and this leads to the TSC2005 never to raise a new  
interrupt again (at least with function 1 in use).

Issue #2 is similar but happens more rarely as it requires more  
precise conditions; if the guest ISR has just finished reading the  
conversion results, i.e. DAV bits are all clear, the SPI read function  
in TSC2005 will immediately clear the interrupt flag. It is then  
possible to get the timer tick callback between this moment and the  
moment when the guest ISR has actually cleared the IRQ. When this  
happens, the callback will try to raise a new interrupt but as the  
guest ISR will clear it shortly thereafter the actual IRQ will be  
cleared but the IRQ flag in the TSC2005 code will stay active causing  
the TSC2005 not to generate new interrupts anymore.

A more elaborate solution than my proposed patch would of course be  
better. I tested my patch only with the "n810" machine emulation with  
an appropriate guest software image which seems to be the only  
configuration using the TSC2005. However I am aware that this  
combination is not utilizing every aspect of the TSC2005.


Regards,
Juha

On Jan 5, 2009, at 07:33, ext andrzej zaborowski wrote:

> 2008/12/19 Riihimaki Juha (Nokia-D-MSW/Helsinki) <address@hidden 
> >:
> > Fixing couple of timing related issues in the TSC2005 emulation.
> >
> > Problems:
> > 1) Setting DAV bits while previous conversion results are being  
> > read results
> > in PINTDAV interrupt line never being released.
> > 2) Altering PINTDAV state during SPI data transfer can result in an
> > inconsistent interrupt state.
>
> Is that because the read happens in two steps (write register number,
> read value)?
>
> The specs say that the DAV bit is not reset, and new conversion
> doesn't start until the result is completely read out, I assumed that
> means it starts after the read command is written, rather than after
> both steps finished.  If this wrong, then I see how this can cause
> problems.
>
> > Solution proposal:
> > Moved PINTDAV altering code from tsc2005_pin_update to  
> > tsc2005_timer_tick
> > and prohibited setting of DAV bits unless PINTDAV is to be set.
>
> You moved the qemu_set_irq to the timer callback while the irq can be
> set/reset in other situations too (e.g. on reset). I thinkt he correct
> thing to do might be to leave tsc2005_pin_update as it and not reset
> the dav bit immediately (adn thus not deassert PINTDAV) until the
> value is also fully read.  Only then we would assign s->dav and call
> tsc2005_pin_update again and start a new conversion.
>
> Same might apply to tsc2102.
>
> Cheers