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

Adapt the CTP7 modules for the OH modifications #138

Open
1 of 2 tasks
lpetre-ulb opened this issue Jul 5, 2019 · 7 comments
Open
1 of 2 tasks

Adapt the CTP7 modules for the OH modifications #138

lpetre-ulb opened this issue Jul 5, 2019 · 7 comments

Comments

@lpetre-ulb
Copy link
Contributor

Brief summary of issue

The temperature measured by the PT100 in the original version of the OH showed a large bias (more than 10°C), at least on some boards. This was traced back to three factors :

  1. The SCA current source only provides nominally 100 µA.
  2. The PT100 does not match well the characteristics of the SCA ADC.
  3. The resistance of the routing corresponds to a few degrees in case of the PT100.

Three decisions have been taken to mitigate this issue :

  1. The 1.8 V voltage measurement input has been converted to a SCA ADC current source measurement input with the help of a precision 1 kOhm resistor +/- 0.05%.
  2. The PT100 has been replaced by a PT1000.
  3. The routing resistance will be measured next Monday on blank PCBs.

Due to timing constraints all the OHs are not modified. Three variants will exists :

  1. Original version with the PT100 and no current measurement.
  2. Partially patched version with the P100, but a precision resistor for current measurement.
  3. Fully patched version with the PT1000 and the precision resistor.

This information will be available in the database.

Another detected issue is the noise. On the following plot the distribution of 250 measurements of the 1 kOhm precision resistor is shown :

Figure_1

It is clear there is a large variation, higher than the 1 LSB quantization error of the SCA ADC. In order to get sensitive data the final value should then average multiple measurements. There is obviously a trade-off between the measurement time and the acceptable uncertainty, so a configurable number of measurement would be ideal.

Types of issue

  • Bug report (report an issue with the code)
  • Feature request (request for change which adds functionality)

Expected Behavior

Theses new informations should be taken into account into the RPC module :

  • The ADC_V1P8 enum value should be renamed and used according to its new real use. The VTTX_CSC_PT100 may be renamed since there are not always PT100.
  • Providing a way to specify the number of measurements per ADC input channel seems important.
@lpetre-ulb lpetre-ulb mentioned this issue Jul 5, 2019
8 tasks
@lpetre-ulb
Copy link
Contributor Author

Here is the datasheet for the new PT1000; it is an Heraeus PRTD SMD 1206 (part 32207595).

The manufacturer does not provide specific calibration, but it follows the DIN EN 60751 2009-05 standard, class F 0.3. Moreover I would recommended to use polynomial approximation for converting the resistance to a temperature (see this webpage for example). The coefficients should be the same for both the PT100 and PT1000.

@mexanick
Copy link
Contributor

What do you think about ADC_V1P8 --> ADC_CUR_SRC and replacing *PT100 to *PT.
Now, considering the number of measurements per ADC input channel - then what should be returned? All measurements, or avg? Please clarify.

@lpetre-ulb
Copy link
Contributor Author

What do you think about ADC_V1P8 --> ADC_CUR_SRC and replacing *PT100 to *PT.

I think that's good. Maybe *PT can be changed to *TEMP if ever PT* are not used on GE2/1, but as you wish.

Now, considering the number of measurements per ADC input channel - then what should be returned? All measurements, or avg? Please clarify.

As we discussed during the meeting sending the median and quartiles is a good idea. Of course it depends on how you want to use the data later.

For example the DAC scans take 100 measurements and return only the average:

//Scan the DAC
uint32_t nReads=100;
for (uint32_t dacVal=dacMin; dacVal<=dacMax; dacVal += dacStep) { //Loop over DAC values
for (unsigned int vfatN=0; vfatN<oh::VFATS_PER_OH; ++vfatN) { //Loop over VFATs
//Skip masked VFATs
if ( !( (notmask >> vfatN) & 0x1)) { //Case: VFAT is masked, skip
//Store word, but with adcVal = 0
unsigned int idx = vfatN*(dacMax-dacMin+1)/dacStep+(dacVal-dacMin)/dacStep;
vec_dacScanData[idx] = ((ohN & 0xf) << 23) + ((vfatN & 0x1f) << 18) + (dacVal & 0xff);
//skip
continue;
} //End Case: VFAT is masked, skip
else{ //Case: VFAT is not masked
//Set DAC value
std::string strDacReg = stdsprintf("GEM_AMC.OH.OH%i.GEB.VFAT%i.",ohN,vfatN) + regName;
writeReg(la, strDacReg, dacVal);
//Read nReads times and take avg value
uint32_t adcVal=0;
for (uint32_t i=0; i<nReads; ++i) {
//Read the ADC
if (foundAdcCached) {
//either reading or writing this register will trigger a cache update
readRawAddress(adcCacheUpdateAddr[vfatN], la->response);
//updating the cache takes 20 us, including a 50% safety factor
std::this_thread::sleep_for(std::chrono::microseconds(20));
}
adcVal += readRawAddress(adcAddr[vfatN], la->response);
}
adcVal = adcVal/nReads;
//Store value
unsigned int idx = vfatN*(dacMax-dacMin+1)/dacStep+(dacVal-dacMin)/dacStep;
vec_dacScanData[idx] = ((ohN & 0xf) << 23) + ((vfatN & 0x1f) << 18) + ((adcVal & 0x3ff) << 8) + (dacVal & 0xff);
} //End Case: VFAT is not masked
} //End Loop over VFATs
} //End Loop over DAC values

@jsturdy
Copy link
Contributor

jsturdy commented Jul 11, 2019

What do you think about ADC_V1P8 --> ADC_CUR_SRC and replacing *PT100 to *PT.

If my understanding is correct that the V1P8 is only changed for a subset of OptoHybrids, I would not simply change the alias... there is nothing preventing having two functions doing completely orthogonal measurements of the same output with a different alias. This is where the careful design should be done, not in the naming of the enums.

@lpetre-ulb
Copy link
Contributor Author

If my understanding is correct that the V1P8 is only changed for a subset of OptoHybrids, I would not simply change the alias... there is nothing preventing having two functions doing completely orthogonal measurements of the same output with a different alias. This is where the careful design should be done, not in the naming of the enums.

Yes V1P8 is only changed for a subset of OptoHybrids. However the V1P8 input is never used for voltage measurement, even on the older OHs. It was foreseen only for the PROM which is not installed so is the 1.8V FEAST.

On the unmodified OHs the input might still be used for current source calibration but that would require careful evaluation. Indeed it is connected to GND with two parallel paths: one from the ADC input to GND through the 1 kOhm resistor and the other one from the ADC input to GND through a 3 kOhm resistor and capacitors. Since DC current should be blocked by the capacitors the current source measurement may give useful results.

In any case changing the alias matches the intend of the ADC input for all OptoHybrids.

@jsturdy
Copy link
Contributor

jsturdy commented Jul 12, 2019

However the V1P8 input is never used for voltage measurement, even on the older OHs. It was foreseen only for the PROM which is not installed so is the 1.8V FEAST.

OK, right, thanks for the reminder.

@lpetre-ulb
Copy link
Contributor Author

As sais in the first post, here is a table with the trace resistance for the different ADC sensors:

PCB ADC_CUR_SRC(Ω) VTTX_GEM_TEMP(Ω) GBT0_TEMP(Ω) VTTX_CSC_TEMP(Ω)
1 0.495 0.389 0.661 1.171
2 0.420 0.446 0.740 1.282
3 0.357 0.323 0.557 1.052
4 0.416 0.357 0.621 1.103

It is clear there is significant board-to-board variation, but it should be taken into account for the PT100. Indeed the bias due to the trace resistance can be as high as ~3°C for the VTTX_CSC_TEMP.

[Yes, V6_FPGA_TEMP is missing. I'll measure it as soon as we have new PCBs.]

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants