STM32 MAC Driver

[steved]

I've just compared the 19.1.3 and trunk support for the Ethernet MAC.

Looking at macWaitTransmitDescriptor() and macWaitReceiveDescriptor(), trunk moves the system lock functions from the LLD to the higher level HAL routines.
The routines in hal_mac.c now hold the system locked while they wait for a descriptor - which doesn't seem right, since the wait could be a significant time.
The 'original' routines potentially did more locking and unlocking, but time spent in the lock zone was minimal and determinate.

What's the thinking behind the change?

[Giovanni]

Hi,

There is a recent bug report, there was a race condition that could make the thread wait with packets already in queue.

viewtopic.php?f=35&t=5368

Don't worry about the critical section, a sleeping thread re-enables interrupts, there is no much difference.

Giovanni

[steved]

Thanks for that.

Looking more closely at hal_mac.c, there's a difference in the two routines I mentioned in that macWaitReceiveDescriptor() doesn't check the timeout value before suspending. Was that intentional?

For now I've changed mine to match macWaitTransmitDescriptor():

Code: Select all

msg_t macWaitReceiveDescriptor(MACDriver *macp,
                               MACReceiveDescriptor *rdp,
                               sysinterval_t timeout) {
  msg_t msg;

  osalDbgCheck((macp != NULL) && (rdp != NULL));
  osalDbgAssert(macp->state == MAC_ACTIVE, "not active");

  osalSysLock();

  while ((((msg = mac_lld_get_receive_descriptor(macp, rdp)) != MSG_OK))  &&
      (timeout > (sysinterval_t)0)) {
    msg = osalThreadEnqueueTimeoutS(&macp->rdqueue, timeout);
    if (msg == MSG_TIMEOUT) {
      break;
    }
  }

  osalSysUnlock();

  return msg;
}


This certainly appears to work OK

[Giovanni]

Hi,

Thanks for finding, that check is a leftover of a very old version, I think it should be removed in both. Look at 17.6, the loop was way more complex.

Fixed as bug #1106.

Giovanni