proper use of time_is_before_jiffies()

Question

My Linux device driver has some obstinate logic which twiddles with some hardware and then waits for a signal to appear. The seemingly proper way is:

            ulong timeout, waitcnt = 0;
            ...

            /* 2.  Establish programming mode */

            gpio_bit_write (MPP_CFG_PROGRAM, 0);  /* assert */
            udelay (3);     /* one microsecond should be long enough */
            gpio_bit_write (MPP_CFG_PROGRAM, 1);  /* de-assert */

            /* 3.  Wait for the FPGA to initialize. */
            /* 100 ms timeout should be nearly 100 times too long */

            timeout = jiffies + msecs_to_jiffies(100);

            while (gpio_bit_read (MPP_CFG_INIT) == 0  &&
                   time_is_before_jiffies (timeout))
                      ++waitcnt;   /* do nothing */

            if (!time_is_before_jiffies (timeout))  /* timed out? */
            {
                  /* timeout error */
            }

This always exercises the "timeout error" path and doesn't increment waitcnt at all. Perhaps I don't understand the meaning of time_is_before_jiffies(), or it is broken. When I replace it with the much more understandable direct comparison of jiffies:

            while (gpio_bit_read (MPP_CFG_INIT) == 0  &&
                   jiffies <= timeout)
                      ++waitcnt;   /* do nothing */

It works just fine: it loops for awhile (1600 µS), sees the INIT bit come on, and then proceeds without triggering a timeout error.

The comment for time_is_before_jiffies() is:

/* time_is_before_jiffies(a) return true if a is before jiffies */
#define time_is_before_jiffies(a) time_after(jiffies, a)

As the sense of the comparison seemed nonsensically backward, I replaced both with time_is_after_jiffies(), but that doesn't work either.

What am I doing wrong? Maybe I should replace use of this confusing macro with the straightforward jiffies <= timeout logic, though that seems less portable.

Answer 1

The jiffies <= timeout comparison does not work when the jiffies are wrapping around, so you must use it.

The condition you want to use can be described as "has not yet timed out". This means that the current time (jiffies) has not yet reached the timeout time (timeout), i.e., jiffies is before the variable you are comparing it to, which means that your variable is after jiffies. (All the time_is_ functions have jiffies on the right side of the comparison.)

So you have to use timer_is_after_jiffies() in the while loop.

(And the <= implies that you actually want to use time_is_after_eq_jiffies().)

The timeout check should be better done by reading the GPIO bit, because it would be a shame if your code times out although it got the signal right at the end. Furthermore, busy-looping for a hundred milliseconds is extremly evil; you should release the CPU if you don't need it:

unsigned long timeout = jiffies + msecs_to_jiffies(100);
bool ok = false;
for (;;) {
    ok = gpio_bit_read(MPP_CFG_INIT) != 0;
    if (ok || time_is_before_eq_jiffies(timeout))
       break;
    /* you should do msleep(...) or cond_resched() here, if possible */
}
if (!ok)  /* timed out? */
    ...

(This loop uses time_is_before_eq_jiffies() because the condition is reversed.)