From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH 3/3] alleviate time drift with HPET periodic timers Date: Tue, 22 Mar 2011 11:08:10 +0100 Message-ID: <4D88750A.4050304@siemens.com> References: <996058219.455940.1300788200730.JavaMail.root@zmail07.collab.prod.int.phx2.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: qemu-devel@nongnu.org, kvm , gcosta@redhat.com, Anthony Liguori , aliguori@us.ibm.com To: Ulrich Obergfell Return-path: Received: from goliath.siemens.de ([192.35.17.28]:20296 "EHLO goliath.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753406Ab1CVKIW (ORCPT ); Tue, 22 Mar 2011 06:08:22 -0400 In-Reply-To: <996058219.455940.1300788200730.JavaMail.root@zmail07.collab.prod.int.phx2.redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 2011-03-22 11:03, Ulrich Obergfell wrote: > >>> Part 3 of the patch implements the following options for the >>> 'configure' script. >>> >>> --disable-hpet-driftfix >>> --enable-hpet-driftfix >> >> I see no benefit in this configurability. Just make the driftfix >> unconditionally available, runtime-disabled by default for now until it >> matured and there is no downside in enabling it all the time. > > > Many Thanks Jan, > > I enclosed the code in '#ifdef CONFIG_HPET_DRIFTFIX ... #endif' > so that it can be easily identified (and removed if the generic API > would be implemented some day). Since the ifdef's are already there > I added the configuration option for convenience. As you don't see > any benefit in this option, I can remove that part of the patch. > However, I'd suggest to keep the ifdef's and do the following: > > - Rename to '#ifdef HPET_DRIFTFIX ... #endif' to make it clear that > this is not controlled via a configuration option. > > - Add '#define HPET_DRIFTFIX' to hw/hpet_emul.h. > > Do you agree ? Thanks to versioning control and feature-oriented commits, it's not very hard to identify what code changes relate to which feature additions. So I still don't see a need for that. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux