Merge pull request #17 from OrangeFox86/16-implement-usb-cdc-device-as-debug-device-instead-of-onboard-uart

16 implement usb cdc device as debug device

Author: Tails86

inc/hal/System/LockGuard.hpp

@@ -0,0 +1,56 @@
+#ifndef __LOCK_GUARD_H__
+#define __LOCK_GUARD_H__
+
+#include <stdint.h>
+#include "MutexInterface.hpp"
+
+//! This class is similar to std::lock_guard, but it is specifically meant to be used with
+//! MutexInterface. The special thing with MutexInterface is that doing a blocking lock has the
+//! potential to cause a deadlock (because spin lock is used, and there is no way to yield from IRQ
+//! context). As such, the functionality using the mutex must check this and perform necessary
+//! operations (even if it's just to log an error and spin forever).
+class LockGuard
+{
+    public:
+        LockGuard() = delete;
+
+        //! Initializes data and locks mutex as long as it wouldn't cause a deadlock
+        inline LockGuard(MutexInterface& mutex) :
+            mMutex(mutex),
+            mIsLocked(false)
+        {
+            int8_t lockValue = mMutex.tryLock();
+            if (lockValue == 0)
+            {
+                mMutex.lock();
+                mIsLocked = true;
+            }
+            else if (lockValue > 0)
+            {
+                mIsLocked = true;
+            }
+            // Else: not locked, would cause deadlock
+        }
+
+        //! Unlocks mutex if it was previously locked
+        virtual inline ~LockGuard()
+        {
+            if (mIsLocked)
+            {
+                mMutex.unlock();
+            }
+        }
+
+        //! @returns true iff the mutex was successfully locked in the constructor or false if
+        //!          locking would cause a deadlock
+        inline bool isLocked()
+        {
+            return mIsLocked;
+        }
+
+    private:
+        MutexInterface& mMutex;
+        bool mIsLocked;
+};
+
+#endif // __LOCK_GUARD_H__

inc/hal/System/MutexInterface.hpp

@@ -1,6 +1,8 @@
 #ifndef __MUTEX_INTERFACE_H__
 #define __MUTEX_INTERFACE_H__
 
+#include <stdint.h>
+
 //! This interface is used to decouple mutex functionality in HAL from the Dreamcast functionality.
 class MutexInterface
 {
@@ -16,6 +18,12 @@ class MutexInterface
 
         //! Releases the previously locked mutex
         virtual void unlock() = 0;
+
+        //! Tries to obtain mutex without blocking
+        //! @returns 1 if mutex was obtained
+        //! @returns 0 if mutex was not obtained, and blocking would be valid in this context
+        //! @returns -1 if mutex was not obtained, and blocking would cause deadlock
+        virtual int8_t tryLock() = 0;
 };
 
 #endif // __MUTEX_INTERFACE_H__

inc/hal/Usb/usb_interface.hpp

@@ -8,7 +8,7 @@
 //! @returns array of the USB controller observers
 DreamcastControllerObserver** get_usb_controller_observers();
 //! USB initialization
-void usb_init();
+void usb_init(MutexInterface* mscMutex, MutexInterface* cdcMutex);
 //! USB task that needs to be called constantly by main()
 void usb_task();
 //! @returns number of USB controllers
@@ -17,8 +17,5 @@ uint32_t get_num_usb_controllers();
 //! Must return the file system
 UsbFileSystem& usb_msc_get_file_system();
 
-//! Set mutex to use between the above two calls and file access
-void usb_msc_set_mutex(MutexInterface* mutex);
-
 
 #endif // __USB_INTERFACE_H__

src/coreLib/ScreenData.cpp

@@ -1,7 +1,8 @@
 #include "ScreenData.hpp"
 #include <string.h>
 #include <assert.h>
-#include <mutex>
+#include "hal/System/LockGuard.hpp"
+#include "utils.h"
 
 ScreenData::ScreenData(MutexInterface& mutex) :
     mMutex(mutex),
@@ -21,9 +22,16 @@ ScreenData::ScreenData(MutexInterface& mutex) :
 void ScreenData::setData(uint32_t* data, uint32_t startIndex, uint32_t numWords)
 {
     assert(startIndex + numWords <= sizeof(mScreenData));
-    std::lock_guard<MutexInterface> lockGuard(mMutex);
-    memcpy(mScreenData + startIndex, data, numWords);
-    mNewDataAvailable = true;
+    LockGuard lockGuard(mMutex);
+    if (lockGuard.isLocked())
+    {
+        memcpy(mScreenData + startIndex, data, numWords);
+        mNewDataAvailable = true;
+    }
+    else
+    {
+        DEBUG_PRINT("FAULT: failed to set screen data\n");
+    }
 }
 
 bool ScreenData::isNewDataAvailable() const
@@ -33,7 +41,15 @@ bool ScreenData::isNewDataAvailable() const
 
 void ScreenData::readData(uint32_t* out)
 {
-    std::lock_guard<MutexInterface> lockGuard(mMutex);
-    mNewDataAvailable = false;
+    LockGuard lockGuard(mMutex);
+    if (lockGuard.isLocked())
+    {
+        mNewDataAvailable = false;
+    }
+    else
+    {
+        DEBUG_PRINT("FAULT: failed to set screen data\n");
+    }
+    // Allow this to happen, even if locking failed
     memcpy(out, mScreenData, sizeof(mScreenData));
 }

src/coreLib/ScreenData.hpp

@@ -9,7 +9,7 @@ class ScreenData
 {
     public:
         //! Constructor
-        //! @param[in] mutex  Reference to the mutex to use
+        //! @param[in] mutex  Reference to the mutex to use (critical section mutex recommended)
         ScreenData(MutexInterface& mutex);
 
         //! Set the screen bits

src/hal/System/CriticalSectionMutex.cpp

@@ -16,3 +16,10 @@ void CriticalSectionMutex::unlock()
 {
     critical_section_exit(&mCriticalSection);
 }
+
+int8_t CriticalSectionMutex::tryLock()
+{
+    // "try" isn't valid for critical section mutex, but taking will never cause deadlock since
+    // interrupts are disabled as part of the locking mechanism.
+    return 0;
+}

src/hal/System/CriticalSectionMutex.hpp

@@ -4,6 +4,13 @@
 #include "hal/System/MutexInterface.hpp"
 #include "pico/critical_section.h"
 
+// From raspberry-pi-pico-c-sdk.pdf:
+// Critical Section API for short-lived mutual exclusion safe for IRQ and multi-core.
+// A critical section is non-reentrant, and provides mutual exclusion using a spin-lock to prevent
+// access from the other core, and from (higher priority) interrupts on the same core. It does the
+// former using a spin lock and the latter by disabling interrupts on the calling core. Because
+// interrupts are disabled when a critical_section is owned, uses of the critical_section should be
+// as short as possible.
 class CriticalSectionMutex : public MutexInterface
 {
     public:
@@ -15,6 +22,8 @@ class CriticalSectionMutex : public MutexInterface
 
         virtual void unlock() final;
 
+        virtual int8_t tryLock() final;
+
     private:
         critical_section_t mCriticalSection;
 };

src/hal/System/Mutex.cpp

@@ -0,0 +1,35 @@
+#include "Mutex.hpp"
+
+Mutex::Mutex()
+{
+    mutex_init(&mMutex);
+}
+
+Mutex::~Mutex() {}
+
+void Mutex::lock()
+{
+    mutex_enter_blocking(&mMutex);
+}
+
+void Mutex::unlock()
+{
+    mutex_exit(&mMutex);
+}
+
+int8_t Mutex::tryLock()
+{
+    uint32_t owner;
+    if (!mutex_try_enter(&mMutex, &owner))
+    {
+        if (owner == get_core_num())
+        {
+            return -1; // would deadlock
+        }
+        else
+        {
+            return 0;
+        }
+    }
+    return 1;
+}

src/hal/System/Mutex.hpp

@@ -0,0 +1,28 @@
+#ifndef __MUTEX_H__
+#define __MUTEX_H__
+
+#include "hal/System/MutexInterface.hpp"
+#include "pico/mutex.h"
+
+// From raspberry-pi-pico-c-sdk.pdf:
+// Mutex API for non IRQ mutual exclusion between cores.
+// Unlike critical sections, the mutex protected code is not necessarily required/expected to
+// complete quickly as no other sytem wide locks are held on account of an acquired mutex.
+class Mutex : public MutexInterface
+{
+    public:
+        Mutex();
+
+        virtual ~Mutex();
+
+        virtual void lock() final;
+
+        virtual void unlock() final;
+
+        virtual int8_t tryLock() final;
+
+    private:
+        mutex_t mMutex;
+};
+
+#endif // __MUTEX_H__