From cabf463627ac2808a3b539f14c95468aa141e912 Mon Sep 17 00:00:00 2001 From: Sylvain Chapeland Date: Tue, 27 Oct 2026 17:20:32 +0100 Subject: [PATCH] ORC-518: add ConfigureOptions and o2-roc-config --clock-config - New ConfigureOptions struct (force, clockConfig), replacing bool force in BarInterface::configure() and CardConfigurator. - o2-roc-config: new option --clock-config=[yes, no, only] to configure the CRU clock (TTC block) separately from the rest. Default yes (unchanged behavior). - README and release notes updated. Co-Authored-By: Claude Opus 5.5 --- README.md | 29 +++++++++++++++ doc/releaseNotes.md | 4 +++ include/ReadoutCard/BarInterface.h | 3 +- include/ReadoutCard/CardConfigurator.h | 5 +-- include/ReadoutCard/ConfigureOptions.h | 41 ++++++++++++++++++++++ src/CardConfigurator.cxx | 10 +++--- src/CommandLineUtilities/ProgramConfig.cxx | 33 +++++++++++++++-- src/Crorc/CrorcBar.cxx | 2 +- src/Crorc/CrorcBar.h | 2 +- src/Cru/CruBar.cxx | 29 ++++++++++----- src/Cru/CruBar.h | 2 +- src/Pda/PdaBar.h | 2 +- 12 files changed, 139 insertions(+), 23 deletions(-) create mode 100644 include/ReadoutCard/ConfigureOptions.h diff --git a/README.md b/README.md index 5e2975f3..c7d2417b 100644 --- a/README.md +++ b/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` diff --git a/doc/releaseNotes.md b/doc/releaseNotes.md index 49161b03..2b01467e 100644 --- a/doc/releaseNotes.md +++ b/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. diff --git a/include/ReadoutCard/BarInterface.h b/include/ReadoutCard/BarInterface.h index 63e552b7..f34e37db 100644 --- a/include/ReadoutCard/BarInterface.h +++ b/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 diff --git a/include/ReadoutCard/CardConfigurator.h b/include/ReadoutCard/CardConfigurator.h index a9aed14e..d0d8d9c5 100644 --- a/include/ReadoutCard/CardConfigurator.h +++ b/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); diff --git a/include/ReadoutCard/ConfigureOptions.h b/include/ReadoutCard/ConfigureOptions.h new file mode 100644 index 00000000..8e1ff448 --- /dev/null +++ b/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_ diff --git a/src/CardConfigurator.cxx b/src/CardConfigurator.cxx index a16112dc..c11b6f47 100644 --- a/src/CardConfigurator.cxx +++ b/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); diff --git a/src/CommandLineUtilities/ProgramConfig.cxx b/src/CommandLineUtilities/ProgramConfig.cxx index bca58c58..b4e9eee4 100644 --- a/src/CommandLineUtilities/ProgramConfig.cxx +++ b/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 #include +#include 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(&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"; diff --git a/src/Crorc/CrorcBar.cxx b/src/Crorc/CrorcBar.cxx index 0c4024ae..56089b30 100644 --- a/src/Crorc/CrorcBar.cxx +++ b/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)); diff --git a/src/Crorc/CrorcBar.h b/src/Crorc/CrorcBar.h index 331fc45d..8f42832f 100644 --- a/src/Crorc/CrorcBar.h +++ b/src/Crorc/CrorcBar.h @@ -52,7 +52,7 @@ class CrorcBar final : public BarInterfaceBase boost::optional 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); diff --git a/src/Cru/CruBar.cxx b/src/Cru/CruBar.cxx index 43867246..9c7bb645 100644 --- a/src/Cru/CruBar.cxx +++ b/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(mClock) == reportInfo.ttcClock && - static_cast(mDownstreamData) == reportInfo.downstreamData && - std::equal(mLinkMap.begin(), mLinkMap.end(), reportInfo.linkMap.begin()) && - checkPonUpstreamStatusExpected(reportInfo.ponStatusRegister, reportInfo.onuAddress) && + bool clockMatches = static_cast(mClock) == reportInfo.ttcClock && //checkClockConsistent(reportInfo.linkMap) && + checkPonUpstreamStatusExpected(reportInfo.ponStatusRegister, reportInfo.onuAddress); + bool othersMatch = static_cast(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(mClock) != reportInfo.ttcClock /*|| !checkClockConsistent(reportInfo.linkMap)*/ || force) { + if (!doClock) { + log("Skipping clock configuration", LogInfoDevel_(4601)); + } else if (static_cast(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(mDownstreamData) != reportInfo.downstreamData || force) { log("Setting downstream data: " + DownstreamData::toString(mDownstreamData), LogInfoDevel_(4603)); ttc.selectDownstreamData(mDownstreamData); diff --git a/src/Cru/CruBar.h b/src/Cru/CruBar.h index 8f0df8b7..b649c1ed 100644 --- a/src/Cru/CruBar.h +++ b/src/Cru/CruBar.h @@ -84,7 +84,7 @@ class CruBar final : public BarInterfaceBase void resetInternalCounters(); void setWrapperCount(); - void configure(bool force = false) override; + void configure(const ConfigureOptions& options) override; Cru::ReportInfo report(bool forConfig = false); Cru::PacketMonitoringInfo monitorPackets(); Cru::TriggerMonitoringInfo monitorTriggers(bool updateable = false); diff --git a/src/Pda/PdaBar.h b/src/Pda/PdaBar.h index 9717fc4f..2065ec79 100644 --- a/src/Pda/PdaBar.h +++ b/src/Pda/PdaBar.h @@ -177,7 +177,7 @@ class PdaBar : public BarInterface return -1; } - void configure(bool /*force*/) override + void configure(const ConfigureOptions& /*options*/) override { std::cout << "Configure invalid through the PDA BAR" << std::endl; }