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

Pgp4TxLiteWrapper: Bug fix in simulation with ASYNC reset #1119

Merged
merged 1 commit into from
Oct 11, 2023

Conversation

lorenzrota
Copy link
Contributor

In Pgp4TxLiteWrapper, changed Pgp4TxLite.phyTxActive from '1' to rstL

Description

When PhyTxActive = '1', Scrambler was never reset, leading to issues after post-syn simulations in ASIC. This is due to the fact that ASIC synthesizer discard initial values assigned during instantiation.

@bengineerd
Copy link
Contributor

It seems like maybe the Scrambler rst in Pgp4TxLite.vhd needs to attach to pgpTxRst?

@ruck314 ruck314 changed the title Added rst to Scrambler Pgp4TxLiteWrapper: Bug fix in simulation with ASYNC reset Oct 11, 2023
@bengineerd
Copy link
Contributor

https://github.com/slaclab/surf/blob/pgp4wrapper-fix/protocols/pgp/pgp4/core/rtl/Pgp4TxLite.vhd#L216-L238

@ruck314 Do you remember why rst is attached to phyTxActiveL here and not pgpTxRst?

@ruck314
Copy link
Contributor

ruck314 commented Oct 11, 2023

@bengineerd TX active typically come up after the TX reset. So the scrambler output is not valid yet until after the GT's TX reset FSM is completed. The idea is to wait until the TX active signal is high before moving information through the protocol pipeline

@bengineerd
Copy link
Contributor

That make sense, though given the way this is received on the other end, it might not matter. The data is going to be lost one way or another. It won't really matter which side of the scrambler that happens on.

@ruck314 ruck314 merged commit 7554480 into pre-release Oct 11, 2023
3 checks passed
@ruck314 ruck314 deleted the pgp4wrapper-fix branch October 11, 2023 19:44
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.

3 participants