clean up pwn and analog

replace built-in atomic with freeRTOS critical functions

Author: hathach

cores/nRF5/HardwarePWM.cpp

@@ -75,10 +75,8 @@ void HardwarePWM::DebugOutput(Stream& logger)
     logger.printf(" || %d:", i);
     if (can_stringify_token(token)) {
       uint8_t * t = (uint8_t*)(&token);
-      static_assert(sizeof(uintptr_t) == 4);
       logger.printf("   \"%c%c%c%c\"", t[0], t[1], t[2], t[3] );
     } else {
-      static_assert(sizeof(uintptr_t) == 4);
       logger.printf(" %08x", token);
     }
     for (size_t j = 0; j < MAX_CHANNELS; j++) {
@@ -96,76 +94,6 @@ void HardwarePWM::DebugOutput(Stream& logger)
 void HardwarePWM::DebugOutput(Stream& logger) {}
 #endif // CFG_DEBUG
 
-// returns true ONLY when (1) no PWM channel has a pin, and (2) the owner token is nullptr
-bool HardwarePWM::takeOwnership(uintptr_t token)
-{
-  bool notInIsr = !isInISR();
-  if (token == 0) {
-    if (notInIsr) {
-      LOG_LV1("HwPWM", "zero / nullptr is not a valid ownership token (attempted use in takeOwnership)");
-    }
-    return false; // cannot take ownership with nullptr
-  }
-  if (token == this->_owner_token) {
-    if (notInIsr) {
-      LOG_LV1("HwPWM", "failing to acquire ownership because already owned by requesting token (cannot take ownership twice)");
-    }
-  }
-  if (this->_owner_token != 0) {
-    return false;
-  }
-  if (this->usedChannelCount() != 0) {
-    return false;
-  }
-  if (this->enabled()) {
-    return false;
-  }
-  // TODO: warn, but do not fail, if taking ownership with IRQs already enabled
-  // NVIC_GetActive
-
-  // Use C++11 atomic CAS operation
-  uintptr_t newValue = 0U;
-  return this->_owner_token.compare_exchange_strong(newValue, token);
-}
-// returns true ONLY when (1) no PWM channel has a pin attached, and (2) the owner token matches
-bool HardwarePWM::releaseOwnership(uintptr_t token)
-{
-  bool notInIsr = !isInISR();
-  if (token == 0) {
-    if (notInIsr) {
-      LOG_LV1("HwPWM", "zero / nullptr is not a valid ownership token (attempted use in releaseOwnership)");
-    }
-    return false;
-  }
-  if (!this->isOwner(token)) {
-    if (notInIsr) {
-      LOG_LV1("HwPWM", "attempt to release ownership when not the current owner");
-    }
-    return false;
-  }
-  if (this->usedChannelCount() != 0) {
-    if (notInIsr) {
-      LOG_LV1("HwPWM", "attempt to release ownership when at least on channel is still connected");
-    }
-    return false;
-  }
-  if (this->enabled()) {
-    if (notInIsr) {
-      LOG_LV1("HwPWM", "attempt to release ownership when PWM peripheral is still enabled");
-    }
-    return false; // if it's enabled, do not allow ownership to be released, even with no pins in use
-  }
-  // TODO: warn, but do not fail, if releasing ownership with IRQs enabled
-  // NVIC_GetActive
-
-  // Use C++11 atomic CAS operation
-  bool result = this->_owner_token.compare_exchange_strong(token, 0U);
-  if (!result) {
-    LOG_LV1("HwPWM", "race condition resulted in failure to acquire ownership");
-  }
-  return result;
-}
-
 HardwarePWM::HardwarePWM(NRF_PWM_Type* pwm) :
   _pwm(pwm)
 {
@@ -350,3 +278,72 @@ uint8_t HardwarePWM::freeChannelCount(void) const
   return MAX_CHANNELS - usedChannelCount();
 }
 
+// returns true ONLY when (1) no PWM channel has a pin, and (2) the owner token is nullptr
+bool HardwarePWM::takeOwnership(uintptr_t token)
+{
+  if (token == 0) {
+    LOG_LV1("HwPWM", "zero / nullptr is not a valid ownership token (attempted use in takeOwnership)");
+    return false;
+  }
+
+  if (token == this->_owner_token) {
+    LOG_LV1("HwPWM", "failing to acquire ownership because already owned by requesting token (cannot take ownership twice)");
+    return false;
+  }
+
+  if (this->_owner_token != 0) return false;
+  if (this->usedChannelCount() != 0) return false;
+  if (this->enabled()) return false;
+
+  if (isInISR())
+  {
+    UBaseType_t intr_status = taskENTER_CRITICAL_FROM_ISR();
+    _owner_token = token;
+    taskEXIT_CRITICAL_FROM_ISR(intr_status);
+  }else
+  {
+    taskENTER_CRITICAL();
+    _owner_token = token;
+    taskEXIT_CRITICAL();
+  }
+
+  return true;
+}
+
+// returns true ONLY when (1) no PWM channel has a pin attached, and (2) the owner token matches
+bool HardwarePWM::releaseOwnership(uintptr_t token)
+{
+  if (token == 0) {
+    LOG_LV1("HwPWM", "zero / nullptr is not a valid ownership token (attempted use in releaseOwnership)");
+    return false;
+  }
+
+  if (!this->isOwner(token)) {
+    LOG_LV1("HwPWM", "attempt to release ownership when not the current owner");
+    return false;
+  }
+
+  if (this->usedChannelCount() != 0) {
+    LOG_LV1("HwPWM", "attempt to release ownership when at least on channel is still connected");
+    return false;
+  }
+
+  if (this->enabled()) {
+    LOG_LV1("HwPWM", "attempt to release ownership when PWM peripheral is still enabled");
+    return false; // if it's enabled, do not allow ownership to be released, even with no pins in use
+  }
+
+  if (isInISR())
+  {
+    UBaseType_t intr_status = taskENTER_CRITICAL_FROM_ISR();
+    _owner_token = 0;
+    taskEXIT_CRITICAL_FROM_ISR(intr_status);
+  }else
+  {
+    taskENTER_CRITICAL();
+    _owner_token = 0;
+    taskEXIT_CRITICAL();
+  }
+
+  return true;
+}

cores/nRF5/HardwarePWM.h

@@ -38,7 +38,6 @@
 
 #include "common_inc.h"
 #include "nrf.h"
-#include <atomic>
 
 #ifdef NRF52840_XXAA
 #define HWPWM_MODULE_NUM    4
@@ -51,7 +50,7 @@ class HardwarePWM
   private:
     enum { MAX_CHANNELS = 4 }; // Max channel per group
     NRF_PWM_Type * const _pwm;
-    std::atomic_uintptr_t _owner_token;
+    uintptr_t _owner_token;
 
     uint16_t _seq0[MAX_CHANNELS];
 
@@ -73,11 +72,12 @@ class HardwarePWM
 
     // returns true ONLY when (1) no PWM channel has a pin, and (2) the owner token is nullptr
     bool takeOwnership   (uintptr_t    token);
+
     // returns true ONLY when (1) no PWM channel has a pin attached, and (2) the owner token matches
     bool releaseOwnership(uintptr_t    token);
 
     // allows caller to verify that they own the peripheral
-    __INLINE bool isOwner(uintptr_t token) const
+    bool isOwner(uintptr_t token) const
     {
       return this->_owner_token == token;
     }

cores/nRF5/wiring_analog.cpp

@@ -59,8 +59,10 @@ void analogWriteResolution( uint8_t res )
   _analogResolution = res;
   for (int i = 0; i<HWPWM_MODULE_NUM; i++)
   {
-    if (!HwPWMx[i]->isOwner(_analogToken)) continue;
-    HwPWMx[i]->setResolution(res);
+    if (HwPWMx[i]->isOwner(_analogToken))
+    {
+      HwPWMx[i]->setResolution(res);
+    }
   }
 }
 
@@ -73,65 +75,50 @@ void analogWriteResolution( uint8_t res )
  */
 void analogWrite( uint32_t pin, uint32_t value )
 {
-  /* If the pin is already in use for analogWrite, this should be fast
-     If the pin is not already in use, then it's OK to take slightly more time to setup
-     */
-
   // first, handle the case where the pin is already in use by analogWrite()
   for(int i=0; i<HWPWM_MODULE_NUM; i++)
   {
-    if (!HwPWMx[i]->isOwner(_analogToken)) {
-      continue; // skip if not owner of this PWM instance
+    if (HwPWMx[i]->isOwner(_analogToken))
+    {
+      int const ch = HwPWMx[i]->pin2channel(pin);
+      if (ch >= 0)
+      {
+        HwPWMx[i]->writeChannel(ch, value);
+        return;
+      }
     }
-
-    int const ch = HwPWMx[i]->pin2channel(pin);
-    if (ch < 0) {
-      continue; // pin not in use by this PWM instance
-    }
-    HwPWMx[i]->writeChannel(ch, value);
-    return;
   }
 
   // Next, handle the case where can add the pin to a PWM instance already owned by analogWrite()
   for(int i=0; i<HWPWM_MODULE_NUM; i++)
   {
-    if (!HwPWMx[i]->isOwner(_analogToken)) {
-      continue;
-    }
-    if (!HwPWMx[i]->addPin(pin)) {
-      continue;
+    if ( HwPWMx[i]->isOwner(_analogToken) && HwPWMx[i]->addPin(pin) )
+    {
+      // successfully added the pin, so write the value also
+      LOG_LV2("Analog", "Added pin %" PRIu32 " to already-owned PWM %d", pin, i);
+      HwPWMx[i]->writePin(pin, value);
+      return;
     }
-
-    // successfully added the pin, so write the value also
-    LOG_LV2("ANA", "Added pin %" PRIu32 " to already-owned PWM %d", pin, i);
-    HwPWMx[i]->writePin(pin, value);
-    return;
   }
 
   // Attempt to acquire a new HwPWMx instance ... but only where
   // 1. it's not one already used for analog, and
   // 2. it currently has no pins in use.
   for(int i=0; i<HWPWM_MODULE_NUM; i++)
   {
-    if (!HwPWMx[i]->takeOwnership(_analogToken)) {
-      continue;
+    if (HwPWMx[i]->takeOwnership(_analogToken))
+    {
+      // apply the cached analog resolution to newly owned instances
+      HwPWMx[i]->setResolution(_analogResolution);
+      HwPWMx[i]->addPin(pin);
+      HwPWMx[i]->writePin(pin, value);
+      LOG_LV2("Analog", "took ownership of, and added pin %" PRIu32 " to, PWM %d", pin, i);
+      return;
     }
-
-    // apply the cached analog resolution to newly owned instances
-    HwPWMx[i]->setResolution(_analogResolution);
-    HwPWMx[i]->addPin(pin);
-    HwPWMx[i]->writePin(pin, value);
-    LOG_LV2("ANA", "took ownership of, and added pin %" PRIu32 " to, PWM %d", pin, i);
-    return;
   }
 
-  // failed to allocate a HwPWM instance.
-  // output appropriate debug message.
-  LOG_LV1("ANA", "Unable to find a free PWM peripheral");
-  // TODO: Add additional diagnostics to function at higher log levels
+  LOG_LV1("Analog", "Unable to find a free PWM peripheral");
   return;
 }
 
 } // end extern "C"
-
-