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

Flip serial_tl_clock to be generated off-chip #1445

Merged
merged 5 commits into from
May 8, 2023
Merged

Conversation

jerryz123
Copy link
Contributor

@jerryz123 jerryz123 commented Apr 18, 2023

Previously, the serial_tl_clock is generated on-chip, and passed to the host bringup FPGA. This flips that, so the FPGA must generate the clock.

Follows up on #1435 to improve the serial_tl robustness and ease-of-use.

Related PRs / Issues:

Type of change:

  • Bug fix
  • New feature
  • Other enhancement

Impact:

  • RTL change
  • Software change (RISC-V software)
  • Build system change
  • Other

Contributor Checklist:

  • Did you set main as the base branch?
  • Is this PR's title suitable for inclusion in the changelog and have you added a changelog:<topic> label?
  • Did you state the type-of-change/impact?
  • Did you delete any extraneous prints/debugging code?
  • Did you mark the PR with a changelog: label?
  • (If applicable) Did you update the conda .conda-lock.yml file if you updated the conda requirements file?
  • (If applicable) Did you add documentation for the feature?
  • (If applicable) Did you add a test demonstrating the PR?
  • (If applicable) Did you mark the PR as Please Backport?

@jerryz123
Copy link
Contributor Author

jerryz123 commented Apr 19, 2023

@harrisonliew You are right, the constraints for the existing vcu118 bringup flow should add a clock input into the chip.

However, doing so would cause backwards compatibility issues with anyone who uses that bringup flow for existing chips. And future chips should use a lightweight bringup platform that is yet-to-be implemented anyways.

Can we just leave the existing vcu118 setup as is, until the new one is brought up?

@abejgonzalez @harrisonliew what do you think?

@harrisonliew
Copy link
Contributor

Can we just leave the existing vcu118 setup as is, until the new one is brought up?

I'm fine with this if either 1) the clock direction is selectable (e.g. via a legacy vs. new VCU118 config) or 2) the VCU118 bringup is explicitly disallowed in the documentation for bringup of chips built using Chipyard version >=1.9.1.

@abejgonzalez
Copy link
Contributor

abejgonzalez commented Apr 20, 2023

I'm fine with deprecating the current VCU118 bringup flow and just adding to the docs "Hey don't use this for future chips use XYZ flow"

Copy link
Contributor

@abejgonzalez abejgonzalez left a comment

Choose a reason for hiding this comment

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

LGTM

@jerryz123
Copy link
Contributor Author

I've realized supporting bidirectional clock is very useful.
This PR will flip the default to be generate-off-clock.
The next PR will add support for bidirectional clock.

@jerryz123 jerryz123 force-pushed the flip_serial_tl branch 5 times, most recently from 720cfe6 to 1114b3b Compare May 8, 2023 01:20
@jerryz123 jerryz123 merged commit 352cc77 into main May 8, 2023
@jerryz123 jerryz123 deleted the flip_serial_tl branch May 8, 2023 15:00
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.

4 participants