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

clash-cores: Fix xpmCdcHandshake #2610

Merged
merged 1 commit into from
Nov 22, 2023
Merged

clash-cores: Fix xpmCdcHandshake #2610

merged 1 commit into from
Nov 22, 2023

Conversation

leonschoorl
Copy link
Member

  • The HDL generated incorrectly set DEST_EXT_HSK=0, configuring it to generate acks automatically. While the exposed API and simulation model assumed external handshaking.
  • The dstStages and srcStages settings were flipped.

Also improved the haddock a bit to clarify what the settings do and relate them to the XPM documentation.

Still TODO:

  • Write a changelog entry (see changelog/README.md), I've skipped this since there is no release or changelog for clash-cores
  • Check copyright notices are up to date in edited files

Copy link
Member

@martijnbastiaan martijnbastiaan left a comment

Choose a reason for hiding this comment

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

Looks good! Could you add the test improvements we made during pair-programming?

 * The HDL generated incorrectly set DEST_EXT_HSK=0,
   configuring it to generate acks automatically.
   While the exposed API and simulation model assumed external handshaking.
 * The dstStages and srcStages settings were flipped.

This also updates the test so it can detect that first error.
And improves the haddock a bit to clarify what the settings do and relate them to the XPM documentation.
@leonschoorl leonschoorl enabled auto-merge (squash) November 22, 2023 14:32
@leonschoorl leonschoorl merged commit 63b90ce into master Nov 22, 2023
12 checks passed
@leonschoorl leonschoorl deleted the fix-xpmCdcHandshake branch November 22, 2023 16:17
mergify bot pushed a commit that referenced this pull request Nov 22, 2023
* The HDL generated incorrectly set DEST_EXT_HSK=0,
   configuring it to generate acks automatically.
   While the exposed API and simulation model assumed external handshaking.
 * The dstStages and srcStages settings were flipped.

This also updates the test so it can detect that first error.
And improves the haddock a bit to clarify what the settings do and relate them to the XPM documentation.

(cherry picked from commit 63b90ce)
martijnbastiaan pushed a commit that referenced this pull request Nov 23, 2023
* The HDL generated incorrectly set DEST_EXT_HSK=0,
   configuring it to generate acks automatically.
   While the exposed API and simulation model assumed external handshaking.
 * The dstStages and srcStages settings were flipped.

This also updates the test so it can detect that first error.
And improves the haddock a bit to clarify what the settings do and relate them to the XPM documentation.

(cherry picked from commit 63b90ce)

Co-authored-by: Leon Schoorl <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants