From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43018) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZAivi-00050D-QH for qemu-devel@nongnu.org; Thu, 02 Jul 2015 14:09:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZAivf-0002s4-Jd for qemu-devel@nongnu.org; Thu, 02 Jul 2015 14:09:46 -0400 Received: from mail-vn0-f51.google.com ([209.85.216.51]:42544) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZAivf-0002rL-FU for qemu-devel@nongnu.org; Thu, 02 Jul 2015 14:09:43 -0400 Received: by vnbf62 with SMTP id f62so12524795vnb.9 for ; Thu, 02 Jul 2015 11:09:42 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <55957A76.3070007@gmail.com> References: <55952A1D.2030806@gmail.com> <1435857603-30613-1-git-send-email-digetx@gmail.com> <55957A76.3070007@gmail.com> From: Peter Maydell Date: Thu, 2 Jul 2015 19:09:23 +0100 Message-ID: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2] arm_mptimer: Fix timer shutdown List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Dmitry Osipenko Cc: Paolo Bonzini , Peter Crosthwaite , QEMU Developers On 2 July 2015 at 18:52, Dmitry Osipenko wrote: > 02.07.2015 20:34, Peter Maydell =D0=BF=D0=B8=D1=88=D0=B5=D1=82: >> >> >> This will now cause us to do the "reload the timer" >> logic if you write a 1 to the control bit when it was >> already 1, which we didn't do before. >> >> The logic I suggested in my previous review >> comment gets this right... >> >> -- PMM >> > > The problem with code you suggested is that won't start periodic count af= ter > one-shot tick was completed. Can you give more detail? This code is only for when the guest writes to the control register, so it doesn't get run when a one-shot tick completes. In any case, the code currently in master does: old value new value action 0 0 nothing 0 1 reload timer 1 0 nothing 1 1 nothing Your first patch did: old value new value action 0 0 delete timer 0 1 reload timer 1 0 delete timer 1 1 nothing Your second patch does: old value new value action 0 0 nothing 0 1 reload timer 1 0 delete timer 1 1 reload timer My suggestion was: old value new value action 0 0 nothing 0 1 reload timer 1 0 delete timer 1 1 nothing If you think that's wrong, then surely your first patch also has the same problem? thanks -- PMM