ipbus / ipbus-firmware

Firmware that implements a reliable high-performance control link for particle physics electronics, based on the IPbus protocol
https://ipbus.web.cern.ch
Other
39 stars 31 forks source link

Support SYSMONs for all SLRs for UltraScale and UltraScale+ #239

Closed jhegeman closed 1 month ago

jhegeman commented 3 months ago

The previous version only instantiated a single SYSMON, even for devices with multiple SLRs. The new implementation allows the user to specify the number of SLRs to populate with SYSMON instances using a VHDL generic. The first SYSMON instance will be placed in the primary SLR, and the other instances will cover the remaining SLRs in order. The instance to address can be chosen with a control register separate from the SYSMON DRP interface.

NOTE: This change is not backwards compatible on purpose. Having to add the generic to the instantiation makes the user of the new (and recommended) functionality. The changes to the address table also require some (minor) software changes.

Over the last weeks, this has been tested on a VU9P (using a VCU118 evaluation board), on a KU15P (using a DTH400-P1V2), and on a VU35P (using a DTH400-P2).

jhegeman commented 2 months ago

Hi. Is there any chance this can be merged, please? Unless the wait is for this to be tested by a third party by implementing it in their builds? (Since this is not covered by the ipbus-firmware tests themselves.)

tswilliams commented 1 month ago

Thanks for opening this PR! I've now tried these changes out on a Serenity with a VU9P daughter card (sorry for the delay), and the changes largely look good to me.

In this Serenity test build though, I got a build error related to placement of the IOBUF, since I didn't have the I2C pins connected to any pins at the top-level in this build. As a workaround I just commented out lines 386 to 399 for that build, but in order to support use cases where the I2C pins aren't connected up, it might be best to either:

Or if you think of other solutions, I'm happy to discuss alternatives.

jhegeman commented 1 month ago

Thanks @tswilliams. I actually had not considered the possibility that anyone would want to use the SYSMON without wiring up the I2C. Now, with the added generic, they can.

tswilliams commented 1 month ago

Run tests