Skip to content
This repository has been archived by the owner on Jan 31, 2022. It is now read-only.

[bugfix] SCA readout update #157

Merged
merged 4 commits into from
Oct 31, 2019
Merged

[bugfix] SCA readout update #157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
16d0d39
remove extraneous copy/paste text
jsturdy Oct 11, 2019
08559f3
add function to check if register name exists, adapt utils to use this
jsturdy Oct 11, 2019
280bf6e
add safety check for missing MONITORING_OFF register using new regExi…
jsturdy Oct 11, 2019
30cf1f3
[bugfix] SCA data format fixed to index the OH properly
jsturdy Oct 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 23 additions & 18 deletions include/amc/sca.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,12 +183,13 @@ void resetSCASEUCounter(const RPCMsg *request, RPCMsg *response);
*
* \param[out] response RPC response message with the following keys:
* - `word_array data` : ADC data is returned as an array of 32-bit words formatted as:
* bit [27]: data present
* bits [26:24]: link ID
* bits [31:29]: constant 0's
* bit [28] : data present
* bits [27:24]: link ID
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering how many OHs will be supported by the foreseen ATCA board? I imagine that it will be more than 16. As a bugfix for the legacy modules, the change it certainly more than enough, but the format should probably be improved when moving to the templated RPC modules.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed, this format should be adapted for an expected future system before it gets into the production stream (templated RPCs)

* bits [23:21]: constant 0's
* bits [20:16]: ADC channel ID
* bits [15:12]: constant 0's
* bits [11:0]: ADC data
* bits [11:0] : ADC data
*/
void readSCAADCSensor(const RPCMsg *request, RPCMsg *response);

Expand All @@ -200,63 +201,67 @@ void readSCAADCSensor(const RPCMsg *request, RPCMsg *response);
*
* \param[out] response RPC response message with the following keys:
* - `word_array data` : ADC data is returned as an array of 32-bit words formatted as:
* bit [27]: data present
* bits [26:24]: link ID
* bits [31:29]: constant 0's
* bit [28] : data present
* bits [27:24]: link ID
* bits [23:21]: constant 0's
* bits [20:16]: ADC channel ID
* bits [15:12]: constant 0's
* bits [11:0]: ADC data
* bits [11:0] : ADC data
*/
void readSCAADCTemperatureSensors(const RPCMsg *request, RPCMsg *response);

/*!
* \fn void readSCAADCVoltageSensors(const RPCMsg *request, RPCMsg *response)
* \brief Read all SCA ADC voltages sensors. They are 1B, 1E, 11, 0E, 18 and 0F.
* \brief Read all SCA ADC voltages sensors. They are 1B, 1E, 11, 0E, 18 and 0F.
* \param[in] request RPC response message with the following keys:
* - `word ohMask` : This specifies which OH's to read from
*
* \param[out] response RPC response message with the following keys:
* - `word_array data` : ADC data is returned as an array of 32-bit words formatted as:
* bit [27]: data present
* bits [26:24]: link ID
* bits [31:29]: constant 0's
* bit [28] : data present
* bits [27:24]: link ID
* bits [23:21]: constant 0's
* bits [20:16]: ADC channel ID
* bits [15:12]: constant 0's
* bits [11:0]: ADC data
* bits [11:0] : ADC data
*/
void readSCAADCVoltageSensors(const RPCMsg *request, RPCMsg *response);

/*!
* \fn void readSCAADCSignalStrengthSensors(const RPCMsg *request, RPCMsg *response)
* \brief Read the SCA ADC signal strength sensors. They are 15, 13 and 12.
* \brief Read the SCA ADC signal strength sensors. They are 15, 13 and 12.
* \param[in] request RPC response message with the following keys:
* - `word ohMask` : This specifies which OH's to read from
*
* \param[out] response RPC response message with the following keys:
* - `word_array data` : ADC data is returned as an array of 32-bit words formatted as:
* bit [27]: data present
* bits [26:24]: link ID
* bits [31:29]: constant 0's
* bit [28] : data present
* bits [27:24]: link ID
* bits [23:21]: constant 0's
* bits [20:16]: ADC channel ID
* bits [15:12]: constant 0's
* bits [11:0]: ADC data
* bits [11:0] : ADC data
*/
void readSCAADCSignalStrengthSensors(const RPCMsg *request, RPCMsg *response);

/*!
* \fn void readAllSCAADCSensors(const RPCMsg *request, RPCMsg *response)
* \brief Read all connected SCA ADC sensors.
* \brief Read all connected SCA ADC sensors.
* \param[in] request RPC response message with the following keys:
* - `word ohMask` : This specifies which OH's to read from
*
* \param[out] response RPC response message with the following keys:
* - `word_array data` : ADC data is returned as an array of 32-bit words formatted as:
* bit [27]: data present
* bits [26:24]: link ID
* bits [31:29]: constant 0's
* bit [28] : data present
* bits [27:24]: link ID
* bits [23:21]: constant 0's
* bits [20:16]: ADC channel ID
* bits [15:12]: constant 0's
* bits [11:0]: ADC data
* bits [11:0] : ADC data
*/
void readAllSCAADCSensors(const RPCMsg *request, RPCMsg *response);

Expand Down
4 changes: 2 additions & 2 deletions include/amc/sca_enums.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ class SCASettings {
enum EADCChannel { //ADCChannel settings
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not technically part of this PR, but is there a specific reason why you preferred enum's in struct's instead of scope enumerations in this file? Using strongly-typed aliases would help preventing bugs and would allow using functions overloads (e.g. replace all sca*Command with a single overloaded scaCommand).

Also not part of this PR, the GE1/1 aliases should be updated to reflect the latest OptoHybrid changes .

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I had used the struct { enum} because there is code in cmsgemos using the same, and it was foreseen that these headers are part of the files that can be included there.
I fully expect that very shortly these will be replaced with the strongly typed enum class syntax, as we're moving to a (more) modern compiler :-)

// ADC 12-bit range
// * Voltage 0-1V (1V/0xfff) LSB, offset 0V
// * Temperature -30-80C (110C/0xfff) LSB, offset -30Cbusiness/SitePages/Fermilab%20Official%20Travel.aspx
// * Temperature -30-80C (110C/0xfff) LSB, offset -30C
AVCCN_V1P0 = 27, ///< FPGA MGT 1.0V
AVTTN_V1P2 = 30, ///< FPGA MGT 1.2V
INT_V1P0 = 17, ///< 1.0V FPGA core voltage
Expand All @@ -234,7 +234,7 @@ class SCASettings {
// SCA ADC temperature sensors
VTTX_CSC_PT100 = 0x00, ///< Transceiver block next to the CSC VTTX
VTTX_GEM_PT100 = 0x04, ///< Transceiver block next to the GEM VTTX
GBT0_PT100 = 0x07, ///< SCA temperature sensor
GBT0_PT100 = 0x07, ///< SCA temperature sensor
V6_FPGA_PT100 = 0x08, ///< Virtex6 temperature sensor

// SCA ADC signal strength sensors
Expand Down
9 changes: 9 additions & 0 deletions include/utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,15 @@ uint32_t bitCheck(uint32_t word, int bit);
*/
uint32_t getNumNonzeroBits(uint32_t value);


/*! \fn bool regExists(LocalArgs * la, const std::string & regName)
* \brief Returns whether or not a named register can be found in the LMDB
* \param la Local arguments structure
* \param regName Register name
* \param db_res pointer to a lmdb::val result that the calling function requires
*/
bool regExists(LocalArgs * la, const std::string & regName, lmdb::val * db_res=nullptr);

/*! \fn uint32_t getMask(LocalArgs * la, const std::string & regName)
* \brief Returns the mask for a given register
* \param la Local arguments structure
Expand Down
74 changes: 50 additions & 24 deletions src/amc/sca.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ uint32_t formatSCAData(uint32_t const& data)

void sendSCACommand(localArgs* la, uint8_t const& ch, uint8_t const& cmd, uint8_t const& len, uint32_t data, uint16_t const& ohMask)
{
// FIXME: DECIDE WHETHER TO HAVE HERE // writeReg(la,"GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF", 0xffffffff);
// FIXME: DECIDE WHETHER TO HAVE HERE // if (regExists(la, "GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF")) {
// FIXME: DECIDE WHETHER TO HAVE HERE // writeReg(la,"GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF", 0xffffffff);
// FIXME: DECIDE WHETHER TO HAVE HERE // }

Comment on lines +21 to +24
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since all calls to sendSCACommand need to be protected against the ADC monitoring code, wouldn't it better to directly move the protection here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think this was my initial proposal as well, but don't remember the reason for not implementing it.
In principle, it can also be removed at some point... but yeah... another time, another PR :-)

writeReg(la,"GEM_AMC.SLOW_CONTROL.SCA.MANUAL_CONTROL.LINK_ENABLE_MASK", ohMask);
writeReg(la,"GEM_AMC.SLOW_CONTROL.SCA.MANUAL_CONTROL.SCA_CMD.SCA_CMD_CHANNEL",ch);
writeReg(la,"GEM_AMC.SLOW_CONTROL.SCA.MANUAL_CONTROL.SCA_CMD.SCA_CMD_COMMAND",cmd);
Expand All @@ -29,8 +32,10 @@ void sendSCACommand(localArgs* la, uint8_t const& ch, uint8_t const& cmd, uint8_

std::vector<uint32_t> sendSCACommandWithReply(localArgs* la, uint8_t const& ch, uint8_t const& cmd, uint8_t const& len, uint32_t data, uint16_t const& ohMask)
{
// FIXME: DECIDE WHETHER TO HAVE HERE // uint32_t monMask = readReg(la,"GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF");
// FIXME: DECIDE WHETHER TO HAVE HERE // writeReg(la,"GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF", 0xffffffff);
// FIXME: DECIDE WHETHER TO HAVE HERE // if (regExists(la, "GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF")) {
// FIXME: DECIDE WHETHER TO HAVE HERE // uint32_t monMask = readReg(la,"GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF");
// FIXME: DECIDE WHETHER TO HAVE HERE // writeReg(la,"GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF", 0xffffffff);
// FIXME: DECIDE WHETHER TO HAVE HERE // }

sendSCACommand(la, ch, cmd, len, data, ohMask);

Expand All @@ -54,8 +59,11 @@ std::vector<uint32_t> sendSCACommandWithReply(localArgs* la, uint8_t const& ch,

std::vector<uint32_t> scaCTRLCommand(localArgs* la, SCACTRLCommandT const& cmd, uint16_t const& ohMask, uint8_t const& len, uint32_t const& data)
{
uint32_t monMask = readReg(la,"GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF");
writeReg(la,"GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF", 0xffffffff);
uint32_t monMask = 0xffffffff;
if (regExists(la, "GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF")) {
monMask = readReg(la,"GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF");
writeReg(la,"GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF", 0xffffffff);
}

std::vector<uint32_t> result;
switch (cmd) {
Expand Down Expand Up @@ -84,7 +92,10 @@ std::vector<uint32_t> scaCTRLCommand(localArgs* la, SCACTRLCommandT const& cmd,
result = sendSCACommandWithReply(la, SCAChannel::CTRL, SCACTRLCommand::GET_DATA, len, data, ohMask);
}

writeReg(la,"GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF", monMask);
if (regExists(la, "GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF")) {
writeReg(la,"GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF", monMask);
}

return result;
}

Expand All @@ -99,32 +110,46 @@ std::vector<uint32_t> scaI2CCommand(localArgs* la, SCAI2CChannelT const& ch, SCA

std::vector<uint32_t> result;

uint32_t monMask = readReg(la,"GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF");
writeReg(la,"GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF", 0xffffffff);
uint32_t monMask = 0xffffffff;
if (regExists(la, "GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF")) {
monMask = readReg(la,"GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF");
writeReg(la,"GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF", 0xffffffff);
}

sendSCACommand(la, ch, cmd, len, data, ohMask);

writeReg(la,"GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF", monMask);
if (regExists(la, "GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF")) {
writeReg(la,"GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF", monMask);
}

return result;
}

std::vector<uint32_t> scaGPIOCommandLocal(localArgs* la, SCAGPIOCommandT const& cmd, uint8_t const& len, uint32_t data, uint16_t const& ohMask)
{
// enable the GPIO bus through the CTRL CRB register, bit 2
uint32_t monMask = readReg(la,"GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF");
writeReg(la,"GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF", 0xffffffff);
uint32_t monMask = 0xffffffff;
if (regExists(la, "GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF")) {
monMask = readReg(la,"GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF");
writeReg(la,"GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF", 0xffffffff);
}

std::vector<uint32_t> reply = sendSCACommandWithReply(la, SCAChannel::GPIO, cmd, len, data, ohMask);

writeReg(la,"GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF", monMask);
if (regExists(la, "GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF")) {
writeReg(la,"GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF", monMask);
}

return reply;
}

std::vector<uint32_t> scaADCCommand(localArgs* la, SCAADCChannelT const& ch, uint16_t const& ohMask)
{
uint32_t monMask = readReg(la,"GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF");
writeReg(la,"GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF", 0xffffffff);
uint32_t monMask = 0xffffffff;
if (regExists(la, "GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF")) {
monMask = readReg(la,"GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF");
writeReg(la,"GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF", 0xffffffff);
}

// enable the ADC bus through the CTRL CRD register, bit 4
// select the ADC channel
Expand All @@ -147,7 +172,9 @@ std::vector<uint32_t> scaADCCommand(localArgs* la, SCAADCChannelT const& ch, uin
// // get the offset
// std::vector<uint32_t> raw = sendSCACommandWithReply(la, SCAChannel::ADC, SCAADCCommand::ADC_R_OFS, 0x1, 0x0, ohMask);

writeReg(la,"GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF", monMask);
if (regExists(la, "GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF")) {
writeReg(la,"GEM_AMC.SLOW_CONTROL.SCA.ADC_MONITORING.MONITORING_OFF", monMask);
}

return result;
}
Expand Down Expand Up @@ -248,16 +275,16 @@ void readSCAADCSensor(const RPCMsg *request, RPCMsg *response)

const uint32_t ohMask = request->get_key_exists("ohMask") ? request->get_word("ohMask") : amc::FULL_OH_MASK;
SCAADCChannelT ch = static_cast<SCAADCChannelT>(request->get_word("ch"));

std::vector<uint32_t> result;
std::vector<uint32_t> outData;

result = scaADCCommand(&la, ch, ohMask);

int ohIdx = 0;
for(auto const& val : result) {
LOGGER->log_message(LogManager::DEBUG, stdsprintf("Value for OH%i, SCA-ADC channel 0x%x = %i ",ohIdx, ch, val));
outData.push_back((bitCheck(ohMask, ohIdx) << 27) | (ohIdx<<24) | (ch<<16) | val);
LOGGER->log_message(LogManager::DEBUG, stdsprintf("Value for OH%i, SCA-ADC channel 0x%x = %i ",ohIdx, ch, val));
outData.push_back((bitCheck(ohMask, ohIdx)<<28) | (ohIdx<<24) | (ch<<16) | val);
response->set_word_array("data",outData);
++ohIdx;
}
Expand Down Expand Up @@ -286,11 +313,11 @@ void readSCAADCTemperatureSensors(const RPCMsg *request, RPCMsg *response)
ohIdx = 0;
for(auto const& val : result) {
LOGGER->log_message(LogManager::DEBUG, stdsprintf("Temperature for OH%i, SCA-ADC channel 0x%x = %i ",ohIdx, channelName, val));
outData.push_back((bitCheck(ohMask, ohIdx) << 27) | (ohIdx<<24) | (channelName<<16) | val);
outData.push_back((bitCheck(ohMask, ohIdx)<<28) | (ohIdx<<24) | (channelName<<16) | val);
++ohIdx;
}
}

response->set_word_array("data", outData);

rtxn.abort();
Expand Down Expand Up @@ -318,7 +345,7 @@ void readSCAADCVoltageSensors(const RPCMsg *request, RPCMsg *response)
ohIdx = 0;
for(auto const& val : result) {
LOGGER->log_message(LogManager::DEBUG, stdsprintf("Voltage for OH%i, SCA-ADC channel 0x%x = %i ",ohIdx, channelName, val));
outData.push_back((bitCheck(ohMask, ohIdx) << 27) | (ohIdx<<24) | (channelName<<16) | val);
outData.push_back((bitCheck(ohMask, ohIdx)<<28) | (ohIdx<<24) | (channelName<<16) | val);
++ohIdx;
}
}
Expand Down Expand Up @@ -348,7 +375,7 @@ void readSCAADCSignalStrengthSensors(const RPCMsg *request, RPCMsg *response)
ohIdx = 0;
for(auto const& val : result) {
LOGGER->log_message(LogManager::DEBUG, stdsprintf("Signal strength for OH%i, SCA-ADC channel 0x%x = %i ",ohIdx, channelName, val));
outData.push_back((bitCheck(ohMask, ohIdx) << 27) | (ohIdx<<24) | (channelName<<16) | val);
outData.push_back((bitCheck(ohMask, ohIdx)<<28) | (ohIdx<<24) | (channelName<<16) | val);
++ohIdx;
}
}
Expand All @@ -372,7 +399,7 @@ void readAllSCAADCSensors(const RPCMsg *request, RPCMsg *response)
ohIdx = 0;
for(auto const& val : result) {
LOGGER->log_message(LogManager::DEBUG, stdsprintf("Reading of OH%i, SCA-ADC channel 0x%x = %i ",ohIdx, channelName, val));
outData.push_back((bitCheck(ohMask, ohIdx) << 27) | (ohIdx<<24) | (channelName<<16) | val);
outData.push_back((bitCheck(ohMask, ohIdx)<<28) | (ohIdx<<24) | (channelName<<16) | val);
++ohIdx;
}
}
Expand All @@ -381,4 +408,3 @@ void readAllSCAADCSensors(const RPCMsg *request, RPCMsg *response)

rtxn.abort();
}

Loading