Fix deep sleep lock bugs

TESTS/mbed_drivers/sleep_lock/main.cpp

@@ -0,0 +1,132 @@
+
+/* mbed Microcontroller Library
+ * Copyright (c) 2017 ARM Limited
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#if !DEVICE_SLEEP
+    #error [NOT_SUPPORTED] Sleep not supported for this target
+#endif
+
+#include "utest/utest.h"
+#include "unity/unity.h"
+#include "greentea-client/test_env.h"
+
+#include "mbed.h"
+
+using namespace utest::v1;
+
+void deep_sleep_lock_lock_test()
+{
+    TEST_ASSERT_EQUAL(true, sleep_manager_can_deep_sleep());
+    {
+        // Check basic usage works
+        DeepSleepLock lock;
+        TEST_ASSERT_EQUAL(false, sleep_manager_can_deep_sleep());
+    }
+
+    TEST_ASSERT_EQUAL(true, sleep_manager_can_deep_sleep());
+    {
+        // Check that unlock and lock change can deep sleep as expected
+        DeepSleepLock lock;
+        TEST_ASSERT_EQUAL(false, sleep_manager_can_deep_sleep());
+        lock.unlock();
+        TEST_ASSERT_EQUAL(true, sleep_manager_can_deep_sleep());
+        lock.lock();
+        TEST_ASSERT_EQUAL(false, sleep_manager_can_deep_sleep());
+    }
+
+    TEST_ASSERT_EQUAL(true, sleep_manager_can_deep_sleep());
+    {
+        // Check that unlock releases sleep based on count
+        DeepSleepLock lock;
+        lock.lock();
+        lock.lock();
+        lock.unlock();
+        TEST_ASSERT_EQUAL(false, sleep_manager_can_deep_sleep());
+    }
+
+    TEST_ASSERT_EQUAL(true, sleep_manager_can_deep_sleep());
+    {
+        // Check that unbalanced locks do not leave deep sleep locked
+        DeepSleepLock lock;
+        lock.lock();
+        TEST_ASSERT_EQUAL(false, sleep_manager_can_deep_sleep());
+    }
+    TEST_ASSERT_EQUAL(true, sleep_manager_can_deep_sleep());
+
+}
+
+void timer_lock_test()
+{
+    TEST_ASSERT_EQUAL(true, sleep_manager_can_deep_sleep());
+    {
+        // Just creating a timer object does not lock sleep
+        Timer timer;
+        TEST_ASSERT_EQUAL(true, sleep_manager_can_deep_sleep());
+    }
+
+    TEST_ASSERT_EQUAL(true, sleep_manager_can_deep_sleep());
+    {
+        // Starting a timer does lock sleep
+        Timer timer;
+        timer.start();
+        TEST_ASSERT_EQUAL(false, sleep_manager_can_deep_sleep());
+    }
+
+    TEST_ASSERT_EQUAL(true, sleep_manager_can_deep_sleep());
+    {
+        // Stopping a timer after starting it allows sleep
+        Timer timer;
+        timer.start();
+        timer.stop();
+        TEST_ASSERT_EQUAL(true, sleep_manager_can_deep_sleep());
+    }
+
+    TEST_ASSERT_EQUAL(true, sleep_manager_can_deep_sleep());
+    {
+        // Starting a timer multiple times still lets you sleep
+        Timer timer;
+        timer.start();
+        timer.start();
+    }
+    TEST_ASSERT_EQUAL(true, sleep_manager_can_deep_sleep());
+
+    TEST_ASSERT_EQUAL(true, sleep_manager_can_deep_sleep());
+    {
+        // Stopping a timer multiple times still lets you sleep
+        Timer timer;
+        timer.start();
+        timer.stop();
+        timer.stop();
+        TEST_ASSERT_EQUAL(true, sleep_manager_can_deep_sleep());
+    }
+    TEST_ASSERT_EQUAL(true, sleep_manager_can_deep_sleep());
+}
+
+Case cases[] = {
+    Case("DeepSleepLock lock test", deep_sleep_lock_lock_test),
+    Case("timer lock test", timer_lock_test),
+};
+
+utest::v1::status_t greentea_test_setup(const size_t number_of_cases) {
+    GREENTEA_SETUP(20, "default_auto");
+    return greentea_test_setup_handler(number_of_cases);
+}
+
+Specification specification(greentea_test_setup, cases, greentea_test_teardown_handler);
+
+int main() {
+    Harness::run(specification);
+}

drivers/CAN.cpp

@@ -46,6 +46,11 @@ CAN::CAN(PinName rd, PinName td, int hz) : _can(), _irq() {
 
 CAN::~CAN() {
     // No lock needed in destructor
+
+    // Detaching interrupts releases the sleep lock if it was locked
+    for (int irq = 0; irq < IrqCnt; irq++) {
+        attach(NULL, (IrqType)irq);
+    }
     can_irq_free(&_can);
     can_free(&_can);
 }

drivers/SerialBase.cpp

@@ -133,6 +133,16 @@ void SerialBase:: unlock() {
     // Stub
 }
 
+SerialBase::~SerialBase()
+{
+    // No lock needed in destructor
+
+    // Detaching interrupts releases the sleep lock if it was locked
+    for (int irq = 0; irq < IrqCnt; irq++) {
+        attach(NULL, (IrqType)irq);
+    }
+}
+
 #if DEVICE_SERIAL_FC
 void SerialBase::set_flow_control(Flow type, PinName flow1, PinName flow2) {
     lock();

drivers/SerialBase.h

@@ -241,8 +241,7 @@ class SerialBase : private NonCopyable<SerialBase> {
 
 protected:
     SerialBase(PinName tx, PinName rx, int baud);
-    virtual ~SerialBase() {
-    }
+    virtual ~SerialBase();
 
     int _base_getc();
     int _base_putc(int c);

drivers/Ticker.h

@@ -22,6 +22,7 @@
 #include "platform/NonCopyable.h"
 #include "platform/mbed_sleep.h"
 #include "hal/lp_ticker_api.h"
+#include "platform/mbed_critical.h"
 
 namespace mbed {
 /** \addtogroup drivers */
@@ -113,12 +114,14 @@ class Ticker : public TimerEvent, private NonCopyable<Ticker> {
      *
      */
     void attach_us(Callback<void()> func, us_timestamp_t t) {
+        core_util_critical_section_enter();
         // lock only for the initial callback setup and this is not low power ticker
         if(!_function && _lock_deepsleep) {
             sleep_manager_lock_deep_sleep();
         }
         _function = func;
         setup(t);
+        core_util_critical_section_exit();
     }
 
     /** Attach a member function to be called by the Ticker, specifying the interval in micro-seconds

platform/DeepSleepLock.h

@@ -16,7 +16,9 @@
 #ifndef MBED_DEEPSLEEPLOCK_H
 #define MBED_DEEPSLEEPLOCK_H
 
+#include <limits.h>
 #include "platform/mbed_sleep.h"
+#include "platform/mbed_critical.h"
 
 namespace mbed {
 
@@ -36,29 +38,47 @@ namespace mbed {
   * @endcode
   */
 class DeepSleepLock {
+private:
+    uint16_t _lock_count;
+
 public:
-    DeepSleepLock()
+    DeepSleepLock(): _lock_count(1)
     {
         sleep_manager_lock_deep_sleep();
     }
 
     ~DeepSleepLock()
     {
-        sleep_manager_unlock_deep_sleep();
+        if (_lock_count) {
+            sleep_manager_unlock_deep_sleep();
+        }
     }
 
     /** Mark the start of a locked deep sleep section
      */
     void lock()
     {
-        sleep_manager_lock_deep_sleep();
+        uint16_t count = core_util_atomic_incr_u16(&_lock_count, 1);
+        if (1 == count) {
+            sleep_manager_lock_deep_sleep();
+        }
+        if (0 == count) {
+            error("DeepSleepLock overflow (> USHRT_MAX)");
+        }
     }
 
     /** Mark the end of a locked deep sleep section
      */
     void unlock()
     {
-        sleep_manager_unlock_deep_sleep();
+        uint16_t count = core_util_atomic_decr_u16(&_lock_count, 1);
+        if (count == 0) {
+            sleep_manager_unlock_deep_sleep();
+        }
+        if (count == USHRT_MAX) {
+            core_util_critical_section_exit();
+            error("DeepSleepLock underflow (< 0)");
+        }
     }
 };