From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:42723)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <chouteau@adacore.com>) id 1Qe8gl-0005zm-93
	for qemu-devel@nongnu.org; Tue, 05 Jul 2011 12:41:32 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <chouteau@adacore.com>) id 1Qe8gj-0003I6-Fe
	for qemu-devel@nongnu.org; Tue, 05 Jul 2011 12:41:30 -0400
Received: from mel.act-europe.fr ([194.98.77.210]:55679)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <chouteau@adacore.com>) id 1Qe8gj-00039h-0A
	for qemu-devel@nongnu.org; Tue, 05 Jul 2011 12:41:29 -0400
Message-ID: <4E133EA9.4040305@adacore.com>
Date: Tue, 05 Jul 2011 18:41:13 +0200
From: Fabien Chouteau <chouteau@adacore.com>
MIME-Version: 1.0
References: <1309529621-27691-1-git-send-email-chouteau@adacore.com>	<20110701152244.278e720b@schlenkerla.am.freescale.net>	<4E11D70E.4070808@adacore.com>
	<20110705110222.1ef56c46@schlenkerla.am.freescale.net>
In-Reply-To: <20110705110222.1ef56c46@schlenkerla.am.freescale.net>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH V2] [PowerPC][RFC] booke timers
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Scott Wood <scottwood@freescale.com>
Cc: qemu-devel@nongnu.org

On 05/07/2011 18:02, Scott Wood wrote:
> On Mon, 4 Jul 2011 17:06:54 +0200
> Fabien Chouteau <chouteau@adacore.com> wrote:
> 
>> On 01/07/2011 22:22, Scott Wood wrote:
>>> On Fri, 1 Jul 2011 16:13:41 +0200
>>> Fabien Chouteau <chouteau@adacore.com> wrote:
>>>> +static void booke_update_fixed_timer(CPUState         *env,
>>>> +                                     uint8_t           target_bit,
>>>> +                                     uint64_t          *next,
>>>> +                                     struct QEMUTimer *timer)
>>>> +{
>>>> +    ppc_tb_t *tb_env = env->tb_env;
>>>> +    uint64_t lapse;
>>>> +    uint64_t tb;
>>>> +    uint64_t period = 1 << (target_bit + 1);
>>>> +    uint64_t now;
>>>> +
>>>> +    now = qemu_get_clock_ns(vm_clock);
>>>> +    tb  = cpu_ppc_get_tb(tb_env, now, tb_env->tb_offset);
>>>> +
>>>> +    if (tb <= (1 << target_bit)) {
>>>> +        lapse = (1 << target_bit) - tb;
>>>> +    } else {
>>>> +        lapse = period - ((tb - (1 << target_bit)) % period);
>>>
>>> We know period is a power of two, so just do "& (period - 1)".
>>>
>>> That should let you get rid of the special case for
>>> "tb <= (1 << target_bit)" as well.
>>>
>>
>> Do you mean "lapse = period - ((tb - (1 << target_bit)) & (period - 1));" ?
> 
> Yes.
> 
> Or more simply:
> 
> lapse = period - ((tb - period) & (period - 1));
> 

Are you sure? Note that period != (1 << target_bit).

>> I don't see how this solves the "tb <= (1 << target_bit)" case.
> 
> Actually, since everything is unsigned the special case shouldn't be needed
> regardless.

You're right about this one, it's tricky though :)

-- 
Fabien Chouteau