Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increased range of SACI_NUM_CHIPS_G to support more than 4 chips #1197

Merged
merged 2 commits into from
Oct 1, 2024

Conversation

hsandber
Copy link
Contributor

Description

The AxiLiteSaciMaster currently has a fixed limit of four ASICs ("chips"). This has been increased to six to work with a new readout system with that many ASICs.

Details

The new 3x2 Readout (https://confluence.slac.stanford.edu/x/eKRxG) seems to be first system to use more than four ASICs with a common SACI interfaces. There is no mention why only four chips were allowed previously. The hardware will arrive in a couple of weeks with which we can test running SACI with six chips, but from the code alone I don't see any reason why more than four would not work.

@ruck314
Copy link
Contributor

ruck314 commented Sep 26, 2024

@hsandber The "why 6 and not 8 or more question"..... What's the upper limit that we could support in the way the code is currently structured?

@hsandber
Copy link
Contributor Author

@ruck314 From the hardware side, the limitations would be from the capacitive loading on the common SACI signals (clk, cmd and rsp). I’m 👍 on removing the limitation altogether from the surf library and leave it to the user to evaluate for each application.

Would probably benefit from a test bench that exercise “large” number of chips and corner cases in that case to make sure nothing else in the module breaks. The current test bench seems to only have one chip.

@ruck314
Copy link
Contributor

ruck314 commented Sep 27, 2024

@hsandber

v.chip := axilWriteMaster.awaddr(22+CHIP_BITS_C-1 downto 22);

Seems like the FW limit is SACI_NUM_CHIPS_G(limit) = 2^(31 -22) = 512, where 31 is the max. AXI-Lite address bit and 22 is the chip address index.

I could imagine in the future that we would need to support 8 (or more) chips on a bus in the future. Should we change this to SACI_NUM_CHIPS_G : positive; with no constraint on the range to match SaciMaster2?
https://github.com/slaclab/surf/blob/main/protocols/saci/rtl/SaciMaster2.vhd#L33
.... Or we could change it to SACI_NUM_CHIPS_G : positive range 1 to 6 := 512; to reflect the actual FW limit

@ruck314 ruck314 changed the title Increased range of SACI_NUM_CHIPS_G from 4 to 6. Increased range of SACI_NUM_CHIPS_G to support more than 4 chips Oct 1, 2024
@ruck314 ruck314 marked this pull request as ready for review October 1, 2024 14:58
@ruck314 ruck314 merged commit de4825d into pre-release Oct 1, 2024
3 checks passed
@ruck314 ruck314 deleted the saci_6_chip branch October 1, 2024 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants