From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53012) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bIhDr-0001N4-5y for qemu-devel@nongnu.org; Thu, 30 Jun 2016 15:02:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bIhDp-0007fp-6H for qemu-devel@nongnu.org; Thu, 30 Jun 2016 15:01:58 -0400 References: <20160625123521.16752-1-digetx@gmail.com> <5814242b-8a8c-852a-51e8-268033b51200@gmail.com> From: Dmitry Osipenko Message-ID: <121dd825-9c64-b3b9-bca8-41bcb984ec0f@gmail.com> Date: Thu, 30 Jun 2016 22:01:45 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] hw/ptimer: Don't wrap around counter for expired timer that uses tick handler List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: QEMU Developers , qemu-arm , Peter Crosthwaite , Mark Cave-Ayland On 30.06.2016 18:02, Peter Maydell wrote: > On 27 June 2016 at 19:26, Dmitry Osipenko wrote: >> On 27.06.2016 16:27, Peter Maydell wrote: >>> I guess this fixes a regression, but it looks really weird. >>> Why should the timer behaviour change if there happens to be >>> a bottom half present? That should be an internal implementation >>> detail. It's also a bit odd that use_icount is in the check: >>> that shouldn't generally affect device emulation behaviour... >> >> In case of a polled timer that doesn't have ptimer trigger bottom half callback >> setup, we are free to wrap around counter since timer behaviour isn't changed >> from ptimer user perspective, as it won't be able to change it's state in the >> handler. >> >> I just decided to keep that wraparound feature for a case of a polled free >> running timer, this should result in a better distribution of the polled value. >> The potential users of that feature are "imx_epit" and "digic" timer device >> models. I should have mentioned it in the commit message to avoid confusion, sorry. >> >> It is still an internal implementation detail, not sure what you are meaning. >> Could you elaborate, please? > > What I meant was: ptimer_get_count() is typically called to generate > a value to return from a register. That's a separate thing, conceptually, > from whether the device happens to also trigger an interrupt on timer > expiry by passing a bh to ptimer_init(). So it's very odd for a detail > of interrupt-on-timer-expiry (that there is a bottom half) to affect > the value returned when you read the timer count register. > In order to handle wraparound correctly, software needs to track the moment of the wraparound - the interrupt. If software reads wrapped around counter value before IRQ triggered (ptimer expired), then it would assume that no wraparound happened and won't perform counter value correction, resulting in periodic counter "jumping" backwards. Anything wrong with it? Am I missing something? -- Dmitry