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

Xilinx US+ Configuration Flow Control Interface incorrect #19

Open
bengiac opened this issue Mar 11, 2024 · 7 comments
Open

Xilinx US+ Configuration Flow Control Interface incorrect #19

bengiac opened this issue Mar 11, 2024 · 7 comments

Comments

@bengiac
Copy link

bengiac commented Mar 11, 2024

I believe _run_cfg_fc_logic for usp_model.py should be combinational instead of clocked to match xilinx behavior

also it appears the _run_cfg_fc_logic is missing the sel == 0000 case

@alexforencich
Copy link
Owner

I wrote that based on the documentation, which TBH isn't really all that clear on the precise behavior. Do you have a simulation or ILA trace that indicates what the behavior should be? You're right that the 000 case is missing, but TBH I'm also not sure if all of the other cases are completely correct. Well, except for TX credit available, which I'm pretty sure is correct (and is the only one that I actually use in designs). And the UltraScale model presumably has the same issue.

Also, I have not bothered to implement cfg_fc_npd and cfg_fc_nph....these signals do not appear to work correctly on the actual hardware (they get stuck at 0), so there isn't really any point in trying to reverse-engineer exactly how they're supposed to work since AFAICT they're completely useless.

@bengiac
Copy link
Author

bengiac commented Mar 11, 2024

thanks for the clarification of the state of these interfaces
I too agree the precise behavior is not clear from the documentation

This is all I have so far, which suggests the selection is combinational, assuming they aren't double registering it
image

@alexforencich
Copy link
Owner

TBH, double registering would make more sense than combinatorial. But I guess either is possible, I think some more ILA captures are necessary, particularly under load so the values can be examined as stuff happens.

@bengiac
Copy link
Author

bengiac commented Mar 11, 2024

appears you are correct, that it is double registered
image

@alexforencich
Copy link
Owner

Hmmmm. It really looks like it's actually only single registered. Right? fc_sel of 0 selects "receive credits available to the link partner", which I suspect reports infinite completion credits (cplh/cpld 0). And that's exactly what you see here: cplh/cpld is zero one cycle after fc_sel is set to 0.

@bengiac
Copy link
Author

bengiac commented Mar 11, 2024

do you mean 2?
fc_sel is set 0 then 2 cycles later cplh/cpld is 0,
possibly my terminology that I use is different to yours,
as I see it,
first capture is fc_sel is held for 2 cycles, and fc_xx changes with it,
second capture is fc_sel held for 4 cycles, and fc_xx is delayed by 2 cycles

@alexforencich
Copy link
Owner

Oh, you're holding it for 2 clock cycles? I guess there is a two-cycle delay, then. It's not super obvious how long things are held for on the ILA trace.

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

No branches or pull requests

2 participants