From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LKpJB-0004NR-7N for qemu-devel@nongnu.org; Thu, 08 Jan 2009 02:28:01 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LKpJ8-0004NF-Mo for qemu-devel@nongnu.org; Thu, 08 Jan 2009 02:27:59 -0500 Received: from [199.232.76.173] (port=58464 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LKpJ8-0004NC-HP for qemu-devel@nongnu.org; Thu, 08 Jan 2009 02:27:58 -0500 Received: from rv-out-0708.google.com ([209.85.198.244]:63956) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1LKpJ8-0006rC-2i for qemu-devel@nongnu.org; Thu, 08 Jan 2009 02:27:58 -0500 Received: by rv-out-0708.google.com with SMTP id f25so8106908rvb.22 for ; Wed, 07 Jan 2009 23:27:55 -0800 (PST) Message-ID: Date: Thu, 8 Jan 2009 08:27:55 +0100 From: "andrzej zaborowski" Subject: Re: [Qemu-devel] [PATCH] TSC2005 interrupt handling fix In-Reply-To: <106AFE1E-1608-4EB4-A7FF-8C5E9DDD1F0F@nokia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <94023A5D-715D-433A-9E23-A5DC1A9A819D@nokia.com> <106AFE1E-1608-4EB4-A7FF-8C5E9DDD1F0F@nokia.com> Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org 2009/1/7 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). The ISR doesn't clear the IRQ -- it can only clear the irq mask flag. If the TSC2005 still keeps the IRQ pin high, then the cpu should immediately jump to the ISR again. We shouldn't be trying to change the tsc2005 emulation so that the driver works, we should change it so that it behaves as described in the specification from TI. I think the real issue is that we start a new conversion before the irq is deasserted, i.e. the following check in tscXXXX_pin_update if (!s->enabled || s->busy) return; should become if (!s->enabled || s->busy || s->dav) return; or something similar. It would be good to check on the real tsc2005 when exactly the DAC becomes busy (bit 14 of CFR0) in the different modes. > > 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. The initial patch you sent was breaking the logic described in the specs so imho it would be a regression. The specs says clearly when the PINT/DAV must be raised and lowered and it's not only in the timer callback. Cheers