STM32 MAC Driver

This forum is dedicated to feedback, discussions about ongoing or future developments, ideas and suggestions regarding the ChibiOS projects are welcome. This forum is NOT for support.
steved
Posts: 719
Joined: Fri Nov 09, 2012 2:22 pm
Has thanked: 10 times
Been thanked: 102 times

STM32 MAC Driver

Postby steved » Fri Jun 19, 2020 10:49 am

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?

User avatar
Giovanni
Site Admin
Posts: 12944
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 731 times
Been thanked: 609 times
Contact:

Re: STM32 MAC Driver

Postby Giovanni » Fri Jun 19, 2020 10:52 am

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
Posts: 719
Joined: Fri Nov 09, 2012 2:22 pm
Has thanked: 10 times
Been thanked: 102 times

Re: STM32 MAC Driver

Postby steved » Fri Jun 19, 2020 12:28 pm

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

User avatar
Giovanni
Site Admin
Posts: 12944
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 731 times
Been thanked: 609 times
Contact:

Re: STM32 MAC Driver

Postby Giovanni » Fri Jun 19, 2020 12:49 pm

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


Return to “Development and Feedback”

Who is online

Users browsing this forum: No registered users and 2 guests