XMC_ETH_MAC_GetPTPTime(): rollover bug?

[MultipleMono]

Hi all, I was going through the implementation of the XMCLib PTP functions, and I think I noticed something that might be a bug.  The XMC_ETH_MAC_GetPTPTime() function is implemented simply as:

time->nanoseconds = (uint32_t)(eth_mac->regs->SYSTEM_TIME_NANOSECONDS); /* accuracy of 1 ns */
time->seconds = eth_mac->regs->SYSTEM_TIME_SECONDS;

 However, it seems like this does not account for the possibility of timer rollover.  Consider the following case:

1. SYSTEM_TIME_NANOSECONDS is 999999980 and SYSTEM_TIME_SECONDS is 0.
2. The first line of the function executes and reads the nanoseconds.
3. A PTP timer tick happens and moves time forward 20ns.    SYSTEM_TIME_NANOSECONDS is now 0 and SYSTEM_TIME_SECONDS is now 1.
4. The second line of the function executes and reads the seconds.
5. The time returns as 1s 999999980ns!

So basically, it seems like there's a very small chance that the time will phantom-jump one second into the future for one call to GetPTPTime().  This would be very rare, it would only happen if a seconds tick happened between the two lines of the function, out of all the 144M instructions executed each second.  However, I think that this is a real issue and I don't see anything in the current code that would defend against it.

I think that the function should be changed to the following:

void XMC_ETH_MAC_GetPTPTime(XMC_ETH_MAC_t *const eth_mac, XMC_ETH_MAC_TIME_t *const time)
{
  XMC_ASSERT("XMC_ETH_MAC_GetPTPTime: eth_mac is invalid", XMC_ETH_MAC_IsValidModule(eth_mac->regs));

  do
  {
      time->nanoseconds = (int32_t)eth_mac->regs->SYSTEM_TIME_NANOSECONDS;
      time->seconds = eth_mac->regs->SYSTEM_TIME_SECONDS;
  }
  // If the time in nanoseconds went down, this means that there was a rollover at some point
  // during the read operation.  Since we don't know if we read SYSTEM_TIME_SECONDS before or after
  // the rollover, we need to retry the read operation.
  // Note that since rollovers happen once a second, this can never loop back more than once.
  while(time->nanoseconds > (int32_t)eth_mac->regs->SYSTEM_TIME_NANOSECONDS);
}

Using a loop like this is a common solution to this sort of problem, and I've used something very similar to fix this same issue reading linked CCU4 timers.

[Vasanth · Accepted solution]

Hi,

Thanks for the suggestions. We will forward this request internally to our software team to look into.


Best Regards,
Vasanth