[roc + crorc] Clean up reset level & fix CRORC bug

Author: kostorr

.gitignore

@@ -6,3 +6,4 @@ cmake-build-*
 *.swp
 *.swo
 *.pyc
+*.patch

include/ReadoutCard/ParameterTypes/ResetLevel.h

@@ -26,10 +26,9 @@ namespace roc
 /// Namespace for the RORC reset level enum, and supporting functions
 struct ResetLevel {
   enum type {
-    Nothing = 0,        ///< No reset
-    Internal = 1,       ///< Reset internally only
-    InternalDiu = 2,    ///< Reset internally, and the DIU
-    InternalDiuSiu = 3, ///< Reset internally, the DIU, and the SIU
+    Nothing = 0,     ///< No reset
+    Internal = 1,    ///< Reset internally only (+DIU for the CRORC)
+    InternalSiu = 2, ///< Reset internally, the DIU, and the SIU (n/a for the CRU)
   };
 
   /// Converts a ResetLevel to a string
@@ -38,7 +37,7 @@ struct ResetLevel {
   /// Converts a string to a ResetLevel
   static ResetLevel::type fromString(const std::string& string);
 
-  /// Returns true if the reset level includes external resets (SIU and/or DIU)
+  /// Returns true if the reset level includes external resets (SIU)
   static bool includesExternal(const ResetLevel::type& level);
 };
 

src/CommandLineUtilities/ProgramReset.cxx

@@ -27,7 +27,7 @@ class ProgramReset : public Program
  public:
   virtual Description getDescription()
   {
-    return { "Reset", "Resets a channel", "roc-reset --id=12345 --channel=0 --reset=INTERNAL_DIU_SIU" };
+    return { "Reset", "Resets a channel", "roc-reset --id=12345 --channel=0 --reset=INTERNAL_SIU" };
   }
 
   virtual void addOptions(boost::program_options::options_description& options)

src/Crorc/Constants.h

@@ -385,7 +385,10 @@ static constexpr Register CHANNEL_RRBARX(0x00000084);
 // Register to send DDL commands
 static constexpr Register DDL_COMMAND(0x00000018); //TODO: Find a better name
 static constexpr Register DDL_STATUS(0x0000001C);
-static constexpr uint32_t SIU_RESET(0x000000f1);
+static constexpr uint32_t SIU_RESET(0x000000F1);
+static constexpr uint32_t RDYRX(0x00000014);
+static constexpr uint32_t STBRD(0x00000054);
+static constexpr uint32_t EOBTR(0x000000B4);
 
 // At bit 13
 static constexpr uint32_t LINK_DOWN(0x00002000);

src/Crorc/CrorcBar.cxx

@@ -235,7 +235,7 @@ void CrorcBar::startTrigger(uint32_t command) //TODO: Rename this
 void CrorcBar::stopTrigger()
 {
   try {
-    sendDdlCommand(Crorc::Registers::DDL_COMMAND.address, Fee::EOBTR);
+    sendDdlCommand(Crorc::Registers::DDL_COMMAND.address, Crorc::Registers::EOBTR);
   } catch (const Exception& e) {
     log("Stopping DDL trigger timed out");
   }
@@ -246,17 +246,19 @@ void CrorcBar::resetCard()
   writeRegister(Crorc::Registers::CRORC_CSR.index, Crorc::Registers::CRORC_RESET);
 }
 
-void CrorcBar::resetDevice()
+void CrorcBar::resetDevice(bool withSiu)
 {
   resetCard();
-  resetSiu();
+  if (withSiu) {
+    resetSiu();
+  }
   assertLinkUp();
 
-  resetSiu();
+  if (withSiu) {
+    resetSiu();
+  }
   resetCard();
   assertLinkUp();
-
-  return;
 }
 
 void CrorcBar::startDataReceiver(uintptr_t readyFifoBusAddress)

src/Crorc/CrorcBar.h

@@ -50,7 +50,7 @@ class CrorcBar final : public BarInterfaceBase
   void configure(bool force = false) override;
   Crorc::ReportInfo report();
 
-  void resetDevice();
+  void resetDevice(bool withSiu);
   void startDataReceiver(uintptr_t readyFifoBusAddress);
   void stopDataReceiver();
   void startTrigger(uint32_t command);

src/Crorc/CrorcDmaChannel.cxx

@@ -40,10 +40,9 @@ namespace roc
 {
 
 CrorcDmaChannel::CrorcDmaChannel(const Parameters& parameters)
-  : DmaChannelPdaBase(parameters, allowedChannels()),                          //
-    mPageSize(parameters.getDmaPageSize().get_value_or(DMA_PAGE_SIZE)),        // 8 kB default for uniformity with CRU
-    mInitialResetLevel(ResetLevel::Internal),                                  // It's good to reset at least the card channel in general
-    mSTBRD(parameters.getStbrdEnabled().get_value_or(false)),                  //TODO: Set as a parameter
+  : DmaChannelPdaBase(parameters, allowedChannels()),
+    mPageSize(parameters.getDmaPageSize().get_value_or(DMA_PAGE_SIZE)), // 8 kB default for uniformity with CRU
+    mSTBRD(parameters.getStbrdEnabled().get_value_or(false)),
     mUseFeeAddress(false),                                                     // Not sure
     mDataSource(parameters.getDataSource().get_value_or(DataSource::Internal)) // Internal loopback by default
 {
@@ -95,7 +94,11 @@ CrorcDmaChannel::CrorcDmaChannel(const Parameters& parameters)
   getReadyFifoUser()->reset();
   mDmaBufferUserspace = getBufferProvider().getAddress();
 
-  deviceResetChannel();
+  if (mDataSource == DataSource::Fee || mDataSource == DataSource::Siu) {
+    deviceResetChannel(ResetLevel::InternalSiu);
+  } else {
+    deviceResetChannel(ResetLevel::Internal);
+  }
 }
 
 auto CrorcDmaChannel::allowedChannels() -> AllowedChannels
@@ -150,7 +153,7 @@ void CrorcDmaChannel::startPendingDma()
       getBar()->assertLinkUp();
 
       // RDYRX command to FEE
-      uint32_t command = (mRDYRX) ? Fee::RDYRX : Fee::STBRD;
+      uint32_t command = (mRDYRX) ? Crorc::Registers::RDYRX : Crorc::Registers::STBRD;
       getBar()->startTrigger(command);
     }
   }
@@ -191,7 +194,9 @@ void CrorcDmaChannel::deviceResetChannel(ResetLevel::type resetLevel)
   if (resetLevel == ResetLevel::Nothing) {
     return;
   }
-  getBar()->resetDevice();
+
+  bool resetSiu = (resetLevel == ResetLevel::InternalSiu) ? true : false;
+  getBar()->resetDevice(resetSiu);
 }
 
 void CrorcDmaChannel::startDataGenerator() //TODO: Update this
@@ -207,7 +212,7 @@ void CrorcDmaChannel::startDataGenerator() //TODO: Update this
     getCrorc().setSiuLoopback(mDiuConfig);
     std::this_thread::sleep_for(100ms); // XXX Why???
     getCrorc().assertLinkUp();
-    getCrorc().siuCommand(Ddl::RandCIFST);
+    getCrorc().siuCommand(Ddl::RandCIFST); //TODO: To be removed
     getCrorc().diuCommand(Ddl::RandCIFST);
   }
 

src/Crorc/CrorcDmaChannel.h

@@ -66,7 +66,7 @@ class CrorcDmaChannel final : public DmaChannelPdaBase
  protected:
   virtual void deviceStartDma() override;
   virtual void deviceStopDma() override;
-  virtual void deviceResetChannel(ResetLevel::type resetLevel = ResetLevel::InternalDiuSiu) override;
+  virtual void deviceResetChannel(ResetLevel::type resetLevel = ResetLevel::InternalSiu) override;
 
  private:
   /// Superpage size supported by the CRORC backend
@@ -180,9 +180,6 @@ class CrorcDmaChannel final : public DmaChannelPdaBase
   /// DMA page size
   const size_t mPageSize;
 
-  /// Reset level on initialization of channel
-  ResetLevel::type mInitialResetLevel;
-
   /// Allows sending the RDYRX and EOBTR commands.
   bool mRDYRX;
 

src/Cru/CruDmaChannel.cxx

@@ -31,7 +31,6 @@ namespace roc
 
 CruDmaChannel::CruDmaChannel(const Parameters& parameters)
   : DmaChannelPdaBase(parameters, allowedChannels()),
-    mInitialResetLevel(ResetLevel::Internal),                                   // It's good to reset at least the card channel in general
     mDataSource(parameters.getDataSource().get_value_or(DataSource::Internal)), // DG loopback mode by default
     mDmaPageSize(parameters.getDmaPageSize().get_value_or(Cru::DMA_PAGE_SIZE))
 {

src/Cru/CruDmaChannel.h

@@ -149,9 +149,6 @@ class CruDmaChannel final : public DmaChannelPdaBase
 
   // These variables are configuration parameters
 
-  /// Reset level on initialization of channel
-  const ResetLevel::type mInitialResetLevel;
-
   /// Gives the data source
   const DataSource::type mDataSource;