Re: [Qemu-devel] [PATCH v6 2/4] cadence_ttc: initial version of device model

[Peter Crosthwaite]

On Tue, Feb 21, 2012 at 11:04 PM, Paul Brook <address@hidden> wrote:
>> > +static inline int64_t is_between(int64_t x, int64_t a, int64_t b)
>> > +{
>> > +    if (a < b) {
>> > +        return x > a && x <= b;
>> > +    }
>> > +    return x < a && x >= b;
>> > +}
>>
>> This looks slightly odd -- should the boundary condition for whether
>> a value equal to the max/min really change depending on :whether a
>> or b is greater?

The function determines whether x is in-between a and b exclusive of
a, inclusive of b, so it is consistent with itself in that regard.

>
> This is a ugly hack.  Instead of figuring out whether we have a count-up or
> count-down timer the code checks for both, and have the "in_between" function
> magically DTRT.  I haven't followed the paths through in enough detail to
> figure out whether it gets all the corner cases right.
>

Is it really a "hack"?? For count up b will always be greater than a,
and for count down the reverse. I suppose I could assert these
conditions at the call site for peace of mind? The invocation from
cadence_timer_run doesn't care whether it is count up of count down,
it really does just only care if the match value is in-between the
current timer value and the next timer value, which is exactly what
this function determines.

> Paul