cms-gem-daq-project / xhal

XHAL interface library
0 stars 10 forks source link

Update trigger OH main monitoring regs #58

Closed mexanick closed 6 years ago

mexanick commented 6 years ago

Modyfying trigger OH main monitoring registers according to changes in PR #29 in ctp7_modules

Description

Types of changes

Motivation and Context

Required for monitoring software to be even with ctp7_modules

How Has This Been Tested?

Screenshots (if appropriate):

Checklist:

evka85 commented 6 years ago

Hi Misha,

If I understand it right, you're swapping out monitoring of LINK{0,1}_NOT_VALID_CNT for: LINK{0,1}_MISSED_COMMA_CNT LINK{0,1}_OVERFLOW_CNT LINK{0,1}_SBIT_OVERFLOW_CNT

This is fine, but you could also add LINK{0,1}_UNDERFLOW_CNT. As we talked before, these counters give different information. But the old LINK{0,1}_NOT_VALID_CNT is actually a subset of LINK{0,1}_MISSED_COMMA_CNT, and thus is redundant and has been removed.

Just to reiterate on the meaning:

I think it's important that the meaning is clear when these counters are reported to the user. Also I think we should reset them during configure state transition (after TCDS has been configured), this can be done by writing to GEM_AMC.TRIGGER.CTRL.CNT_RESET.

Cheers, Evaldas

mexanick commented 6 years ago

missing registers are added

bdorney commented 6 years ago

I guess a separate piece of code will be raising a warning or error state depending on the output of these register reads shown here?

Additionally the same would be true for @evka85's suggestion:

Also I think we should reset them during configure state transition (after TCDS has been configured), this can be done by writing to GEM_AMC.TRIGGER.CTRL.CNT_RESET.

This PR covers only the monitoring of these registers not the follow up action once their values are known, correct?

mexanick commented 6 years ago

Yes. The follow up action will be added later (perhaps to daq_suite, but possibly to another piece of SW)