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

[pattgen,rtl] pattgen_chan cleanup #26520

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

hcallahan-lowrisc
Copy link
Contributor

Motivated by reading the code while trying to review #26319 and interpret the vague testpoint description. This change clarifies the implementation of a pattgen channel to better match up with the spec documentation.

No functional change. The first two commits are minor RTL refactors, while the third extends the documentation comments throughout the new changes.

The documentation describes a FSM which does not exist in a canonical form within the module, but is practically implemented using the 'active' signal. Add a documentation comment to the code explaining this. I guess an alternative would be to change the RTL, but I wanted to keep the changes minimal for this changeset.

Cleanup these two related signals into seperate functional blocks, in
preparation for an upcoming documentation commit.

Signed-off-by: Harry Callahan <[email protected]>
This conditional is used thrice, so factor it out into a sensibly named signal.
This will also gain a documentation comment in an upcoming commit.

Signed-off-by: Harry Callahan <[email protected]>
Copy link
Contributor

@alees24 alees24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. LGTM.

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