Merge pull request #433 from sy-c/orc-518

ORC-518: add ConfigureOptions and o2-roc-config --clock-config

Author: sy-c

README.md

@@ -175,6 +175,21 @@ The `CardConfigurator` class offers an interface to configure the Readout Card.
 order to configure the CRU or CRORC, one has to create a `CardConfigurator` object. The constructor can either be called with a list of
 parameters, or a path to a configuration file, specifying these parameters.
 
+The constructor also takes an optional `ConfigureOptions` struct (see [ConfigureOptions.h](include/ReadoutCard/ConfigureOptions.h)),
+which controls *how* the configuration is applied, as opposed to the `Parameters`, which define *what* is configured:
+
+| Field         | Values                 | Default | Description                                                                                       |
+| ------------- | ---------------------- | ------- | ------------------------------------------------------------------------------------------------- |
+| `force`       | `true`, `false`        | `false` | Rewrite all settings, even if the card state already matches the requested configuration         |
+| `clockConfig` | `Yes`, `No`, `Only`    | `Yes`   | CRU clock (TTC) configuration: run it with the rest, skip it, or run only the clock configuration |
+
+```cpp
+ConfigureOptions options;
+options.force = true;
+options.clockConfig = ConfigureOptions::ClockConfig::No;
+CardConfigurator(parameters, options);
+```
+
 ### CRU Parameters
 
 The `CardConfigurator` utilizes the `Parameters` class, the same class where Parameters are specified for DMA channels. For the
@@ -394,6 +409,20 @@ running instance of readout.exe or roc-bench-dma fail.
 ### roc-config
 Configures the CRU. Can be executed with a list of parameters, or with a [configuration file](#configuration-file). Uses the [Card Configurator](#card-configurator). For more details refer to the `--help` dialog of the binary.
 
+The `--clock-config` option (`yes`, `no`, `only`; default `yes`) sets when the CRU clock is configured. The clock
+configuration (clock selection, TTC/PON calibration and GBT calibration) is slow, so it can be done separately from the
+rest of the configuration:
+* `yes`: the clock is configured together with the other settings.
+* `no`: the clock configuration is skipped, and only the other settings are configured.
+* `only`: only the clock is configured, and the other settings are left untouched.
+
+When `--force` is not set, only the settings selected by `--clock-config` are compared with the current card state to decide
+whether reconfiguring is needed. Example:
+```
+o2-roc-config --id=b0:00.0 --tf=32 --clock-config=only   # configure the clock
+o2-roc-config --id=b0:00.0 --tf=32 --clock-config=no     # configure the rest, without touching the clock
+```
+
 ### roc-example
 The compiled example of `src/Example.cxx`
  

doc/releaseNotes.md

@@ -103,3 +103,7 @@ This file describes the main feature changes for released versions of ReadoutCar
 ## v0.46.2 - 03/03/2026
 - boost compatibility
 - removed some obsolete CRU BAR code
+
+## next
+- o2-roc-config: added option --clock-config=[yes, no, only] to configure the CRU clock separately from the rest, cf [ORC-518](https://its.cern.ch/jira/browse/ORC-518). Default yes (unchanged behavior).
+- BarInterface::configure() and CardConfigurator now take a ConfigureOptions struct (force, clockConfig) instead of bool force.

include/ReadoutCard/BarInterface.h

@@ -22,6 +22,7 @@
 #include "ReadoutCard/CardType.h"
 #include "ReadoutCard/RegisterReadWriteInterface.h"
 #include "ReadoutCard/Parameters.h"
+#include "ReadoutCard/ConfigureOptions.h"
 #include "boost/optional.hpp"
 
 namespace o2
@@ -76,7 +77,7 @@ class BarInterface : public virtual RegisterReadWriteInterface
 
   virtual int getEndpointNumber() = 0;
 
-  virtual void configure(bool force) = 0;
+  virtual void configure(const ConfigureOptions& options) = 0;
 };
 
 } // namespace roc

include/ReadoutCard/CardConfigurator.h

@@ -20,6 +20,7 @@
 #include "ReadoutCard/NamespaceAlias.h"
 #include "ReadoutCard/CardType.h"
 #include "ReadoutCard/Parameters.h"
+#include "ReadoutCard/ConfigureOptions.h"
 
 namespace o2
 {
@@ -29,8 +30,8 @@ namespace roc
 class CardConfigurator
 {
  public:
-  CardConfigurator(Parameters::CardIdType cardId, std::string pathToConfigFile, bool forceConfigure = false);
-  CardConfigurator(Parameters& parameters, bool forceConfigure = false);
+  CardConfigurator(Parameters::CardIdType cardId, std::string pathToConfigFile, const ConfigureOptions& options = {});
+  CardConfigurator(Parameters& parameters, const ConfigureOptions& options = {});
 
  private:
   void parseConfigUri(CardType::type cardType, std::string configUri, Parameters& parameters);

include/ReadoutCard/ConfigureOptions.h

@@ -0,0 +1,41 @@
+// Copyright 2019-2020 CERN and copyright holders of ALICE O2.
+// See https://alice-o2.web.cern.ch/copyright for details of the copyright holders.
+// All rights not expressly granted are reserved.
+//
+// This software is distributed under the terms of the GNU General Public
+// License v3 (GPL Version 3), copied verbatim in the file "COPYING".
+//
+// In applying this license CERN does not waive the privileges and immunities
+// granted to it by virtue of its status as an Intergovernmental Organization
+// or submit itself to any jurisdiction.
+/// \file ConfigureOptions.h
+/// \brief Definition of the ConfigureOptions struct.
+///
+/// \author Sylvain Chapeland (sylvain.chapeland@cern.ch)
+
+#ifndef O2_READOUTCARD_INCLUDE_CONFIGUREOPTIONS_H_
+#define O2_READOUTCARD_INCLUDE_CONFIGUREOPTIONS_H_
+
+#include "ReadoutCard/NamespaceAlias.h"
+
+namespace o2
+{
+namespace roc
+{
+
+/// Options controlling how a card configuration is applied (not what is configured: see Parameters)
+struct ConfigureOptions {
+  /// Rewrite all settings, even if the card state already matches
+  bool force = false;
+
+  /// Clock (TTC) configuration block: run it, skip it, or run only it
+  enum class ClockConfig { Yes,
+                           No,
+                           Only };
+  ClockConfig clockConfig = ClockConfig::Yes;
+};
+
+} // namespace roc
+} // namespace o2
+
+#endif // O2_READOUTCARD_INCLUDE_CONFIGUREOPTIONS_H_

src/CardConfigurator.cxx

@@ -26,7 +26,7 @@ namespace o2
 namespace roc
 {
 
-CardConfigurator::CardConfigurator(Parameters::CardIdType cardId, std::string configUri, bool forceConfigure)
+CardConfigurator::CardConfigurator(Parameters::CardIdType cardId, std::string configUri, const ConfigureOptions& options)
 {
   auto cardType = RocPciDevice(cardId).getCardDescriptor().cardType;
   Parameters parameters;
@@ -47,23 +47,23 @@ CardConfigurator::CardConfigurator(Parameters::CardIdType cardId, std::string co
 
   auto bar2 = ChannelFactory().getBar(parameters);
   try {
-    bar2->configure(forceConfigure);
+    bar2->configure(options);
   } catch (const Exception& e) {
     BOOST_THROW_EXCEPTION(e);
   }
 }
 
-CardConfigurator::CardConfigurator(Parameters& parameters, bool forceConfigure)
+CardConfigurator::CardConfigurator(Parameters& parameters, const ConfigureOptions& options)
 {
   auto cardType = RocPciDevice(parameters.getCardId().get()).getCardDescriptor().cardType;
   try {
     if (cardType == CardType::Cru) {
       auto bar2 = ChannelFactory().getBar(parameters);
-      bar2->configure(forceConfigure);
+      bar2->configure(options);
     } else if (cardType == CardType::Crorc) {
       parameters.setChannelNumber(0); // we need to change to bar 0
       auto bar0 = ChannelFactory().getBar(parameters);
-      bar0->configure(forceConfigure);
+      bar0->configure(options);
     }
   } catch (const Exception& e) {
     BOOST_THROW_EXCEPTION(e);

src/CommandLineUtilities/ProgramConfig.cxx

@@ -22,12 +22,14 @@
 #include "Crorc/CrorcBar.h"
 #include "ReadoutCard/CardConfigurator.h"
 #include "ReadoutCard/ChannelFactory.h"
+#include "ReadoutCard/ConfigureOptions.h"
 #include "ReadoutCard/Exception.h"
 #include "ReadoutCard/FirmwareChecker.h"
 #include "ReadoutCard/Logger.h"
 #include "RocPciDevice.h"
 #include <InfoLogger/InfoLogger.hxx>
 #include <boost/format.hpp>
+#include <boost/algorithm/string/predicate.hpp>
 
 using namespace AliceO2::InfoLogger;
 using namespace o2::roc::CommandLineUtilities;
@@ -330,6 +332,9 @@ class ProgramConfig : public Program
     options.add_options()("drop-bad-rdh",
                           po::bool_switch(&mOptions.dropBadRdhEnabled),
                           "Flag to enable dropping of packets with bad RDH");
+    options.add_options()("clock-config",
+                          po::value<std::string>(&mOptions.clockConfig)->default_value("yes"),
+                          "Clock (TTC) configuration [yes, no, only]: yes = configure clock with the rest (default), no = skip clock configuration, only = configure only the clock");
     options.add_options()("test-mode-ORC501",
                           po::bool_switch(&o2::roc::testModeORC501),
                           "Flag to enable test mode as described in JIRA ORC-501");
@@ -430,11 +435,32 @@ class ProgramConfig : public Program
 
   const char* ilFacility = "ReadoutCard/config";
 
+  /// Converts the --clock-config option value
+  static ConfigureOptions::ClockConfig clockConfigFromString(const std::string& value)
+  {
+    if (boost::iequals(value, "yes")) {
+      return ConfigureOptions::ClockConfig::Yes;
+    } else if (boost::iequals(value, "no")) {
+      return ConfigureOptions::ClockConfig::No;
+    } else if (boost::iequals(value, "only")) {
+      return ConfigureOptions::ClockConfig::Only;
+    }
+    BOOST_THROW_EXCEPTION(Exception() << ErrorInfo::Message("Invalid --clock-config value: " + value + " (expected yes, no or only)"));
+  }
+
   virtual void run(const boost::program_options::variables_map& map)
   {
 
     Logger::setFacility(ilFacility);
 
+    // Options controlling how the configuration is applied
+    ConfigureOptions configureOptions;
+    configureOptions.force = mOptions.forceConfig;
+    configureOptions.clockConfig = clockConfigFromString(mOptions.clockConfig);
+    if (configureOptions.clockConfig != ConfigureOptions::ClockConfig::Yes) {
+      Logger::get() << "`--clock-config` set to " << mOptions.clockConfig << LogDebugDevel_(4600) << endm;
+    }
+
     // Configure all cards found - Normally used during boot
     if (mOptions.configAll) {
       Logger::get() << "Running RoC Configuration for all cards" << LogInfoDevel_(4600) << endm;
@@ -451,7 +477,7 @@ class ProgramConfig : public Program
         if (!mOptions.bypassFirmwareCheck) {
           try {
             FirmwareChecker().checkFirmwareCompatibility(params);
-            CardConfigurator(card.pciAddress, mOptions.configUri, mOptions.forceConfig);
+            CardConfigurator(card.pciAddress, mOptions.configUri, configureOptions);
             reportStatus(card.pciAddress);
           } catch (const std::runtime_error& e) {
             Logger::get() << e.what() << LogErrorDevel_(4600) << endm;
@@ -549,7 +575,7 @@ class ProgramConfig : public Program
       }
 
       try {
-        CardConfigurator(params, mOptions.forceConfig);
+        CardConfigurator(params, configureOptions);
         reportStatus(cardId);
       } catch (const std::runtime_error& e) {
         Logger::get() << e.what() << LogErrorDevel_(4600) << endm;
@@ -565,7 +591,7 @@ class ProgramConfig : public Program
       }
 
       try {
-        CardConfigurator(cardId, mOptions.configUri, mOptions.forceConfig);
+        CardConfigurator(cardId, mOptions.configUri, configureOptions);
         reportStatus(cardId);
       } catch (const std::runtime_error& e) {
         Logger::get() << e.what() << LogErrorDevel_(4600) << endm;
@@ -580,6 +606,7 @@ class ProgramConfig : public Program
 
   struct OptionsStruct {
     std::string clock = "local";
+    std::string clockConfig = "yes";
     std::string configUri = "";
     std::string datapathMode = "packet";
     std::string downstreamData = "Ctp";

src/Crorc/CrorcBar.cxx

@@ -104,7 +104,7 @@ int CrorcBar::getEndpointNumber()
   return 0;
 }
 
-void CrorcBar::configure(bool /*force*/)
+void CrorcBar::configure(const ConfigureOptions& /*options*/)
 {
   log("Configuring...", LogInfoDevel_(4650));
 

src/Crorc/CrorcBar.h

@@ -52,7 +52,7 @@ class CrorcBar final : public BarInterfaceBase
   boost::optional<int32_t> getSerialNumber();
   void setSerial(int serial);
 
-  void configure(bool force = false) override;
+  void configure(const ConfigureOptions& options) override;
   Crorc::ReportInfo report(bool forConfig = false);
 
   void resetDevice(bool withSiu);

src/Cru/CruBar.cxx

@@ -726,17 +726,21 @@ void CruBar::checkConfigParameters()
 }
 
 /// Configures the CRU according to the parameters passed on init
-void CruBar::configure(bool force)
+void CruBar::configure(const ConfigureOptions& options)
 {
+  bool force = options.force;
+  bool doClock = options.clockConfig != ConfigureOptions::ClockConfig::No;      // run the TTC block
+  bool onlyClock = options.clockConfig == ConfigureOptions::ClockConfig::Only; // run only the TTC block
+
   // Get current info
   Cru::ReportInfo reportInfo = report(true);
   populateLinkMap(mLinkMap);
 
-  if (static_cast<uint32_t>(mClock) == reportInfo.ttcClock &&
-      static_cast<uint32_t>(mDownstreamData) == reportInfo.downstreamData &&
-      std::equal(mLinkMap.begin(), mLinkMap.end(), reportInfo.linkMap.begin()) &&
-      checkPonUpstreamStatusExpected(reportInfo.ponStatusRegister, reportInfo.onuAddress) &&
+  bool clockMatches = static_cast<uint32_t>(mClock) == reportInfo.ttcClock &&
       //checkClockConsistent(reportInfo.linkMap) &&
+      checkPonUpstreamStatusExpected(reportInfo.ponStatusRegister, reportInfo.onuAddress);
+  bool othersMatch = static_cast<uint32_t>(mDownstreamData) == reportInfo.downstreamData &&
+      std::equal(mLinkMap.begin(), mLinkMap.end(), reportInfo.linkMap.begin()) &&
       mCruId == reportInfo.cruId &&
       mDynamicOffset == reportInfo.dynamicOffset &&
       mTriggerWindowSize == reportInfo.triggerWindowSize &&
@@ -745,8 +749,10 @@ void CruBar::configure(bool force)
       mRunStatsEnabled == reportInfo.runStatsEnabled &&
       mGbtEnabled == reportInfo.gbtEnabled &&
       mTimeFrameLength == reportInfo.timeFrameLength &&
-      mDropBadRdhEnabled == reportInfo.dropBadRdhEnabled &&
-      !force) {
+      mDropBadRdhEnabled == reportInfo.dropBadRdhEnabled;
+
+  // only compare the parts selected for configuration
+  if ((!doClock || clockMatches) && (onlyClock || othersMatch) && !force) {
     log("No need to reconfigure further", LogInfoDevel_(4600));
     return;
   }
@@ -758,7 +764,9 @@ void CruBar::configure(bool force)
   DatapathWrapper datapathWrapper = DatapathWrapper(mPdaBar);
 
   /* TTC */
-  if (static_cast<uint32_t>(mClock) != reportInfo.ttcClock /*|| !checkClockConsistent(reportInfo.linkMap)*/ || force) {
+  if (!doClock) {
+    log("Skipping clock configuration", LogInfoDevel_(4601));
+  } else if (static_cast<uint32_t>(mClock) != reportInfo.ttcClock /*|| !checkClockConsistent(reportInfo.linkMap)*/ || force) {
     log("Setting the clock to " + Clock::toString(mClock), LogInfoDevel_(4601));
     ttc.setClock(mClock);
 
@@ -787,6 +795,11 @@ void CruBar::configure(bool force)
     }
   }
 
+  if (onlyClock) {
+    log("CRU clock configuration done", LogInfoDevel_(4600));
+    return;
+  }
+
   if (static_cast<uint32_t>(mDownstreamData) != reportInfo.downstreamData || force) {
     log("Setting downstream data: " + DownstreamData::toString(mDownstreamData), LogInfoDevel_(4603));
     ttc.selectDownstreamData(mDownstreamData);