Re: [PATCH] qemu: aspeed_timer: Use signed muldiv for timer resets

From: Christian Svensson <blue@cmd.nu>
To: andrew@aj.id.au
Cc: clg@kaod.org, Christian Svensson <bluecmd@google.com>,
	 "openbmc @ lists . ozlabs . org" <openbmc@lists.ozlabs.org>,
	Joel Stanley <joel@jms.id.au>
Subject: Re: [PATCH] qemu: aspeed_timer: Use signed muldiv for timer resets
Date: Tue, 13 Nov 2018 09:58:08 +0100	[thread overview]
Message-ID: <CADiuDARPsPGkHP1HRm1RVP=G2scSLozSON3o5h_PuA3ngT5eXg@mail.gmail.com> (raw)
In-Reply-To: <1542068863.1552891.1574692280.790D270F@webmail.messagingengine.com>

[-- Attachment #1: Type: text/plain, Size: 1000 bytes --]

On Tue, Nov 13, 2018 at 1:27 AM Andrew Jeffery <andrew@aj.id.au> wrote:
> By inspection the concept of the patch seems okay to me. However, the
issue
> that host-utils.h takes care of in addition to providing helpers is
support for
> those helpers on systems that don't support 128-bit integers. The patch
should
> be sent to the upstream lists, and I don't think we should be breaking
> qemu-arm-* generally for systems that we might not care about.

Ah, yes - I missed that there were two - one for systems with int128 and
one for them
without. Hm, that does complicate things.

Given that we only have one operand that is ever negative, how do you feel
about a patch
that goes along the lines of:

if (delta >= 0) {
  t->start += muldiv64(delta, NANOSECONDS_PER_SECOND, rate);
} else {
   t->start = (int64_t)t->start - muldiv64(-delta, NANOSECONDS_PER_SECOND,
rate);
}

That should avoid any issues I think, save us/me from implementing a signed
muldiv,
but cost a bit on the readable code side.

[-- Attachment #2: Type: text/html, Size: 1320 bytes --]