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

FIX: Change default UART FIFO size and combine UART Config fragments #1683

Merged
merged 12 commits into from
Mar 7, 2024

Conversation

T-K-233
Copy link
Member

@T-K-233 T-K-233 commented Dec 4, 2023

Many tapeout chips are using the default WithUART config in the AbstractConfig. In WithUART, the TX and RX FIFO depth is set to 256, which is an unrealistically large value. Moreover, it makes UART debugging very difficult, since the FIFO is not flushed when chip is under reset.

This PR changes the default FIFO depth to 8, which is the value used on FE310.

Additionally, since now we have control over FIFO depth in the WithUART config fragment, the WithUARTFIFOEntries is redundant. This PR changes the code that references WithUARTFIFOEntries to WithUART.

The WithUARTFIFOEntries is marked deprecated, and is recommended to be removed in future releases.

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?

@T-K-233 T-K-233 requested a review from jerryz123 December 4, 2023 10:39
@T-K-233 T-K-233 changed the title Change default UART FIFO size and combine UART Config fragments FIX: Change default UART FIFO size and combine UART Config fragments Dec 4, 2023
@jerryz123
Copy link
Contributor

@abejgonzalez Does this have any implications for Firesim?

@T-K-233 T-K-233 requested a review from abejgonzalez January 2, 2024 19:13
@T-K-233 T-K-233 requested a review from abejgonzalez March 6, 2024 21:24
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.

SFTM.

@jerryz123 jerryz123 merged commit 44693de into main Mar 7, 2024
54 checks passed
@jerryz123 jerryz123 deleted the uart-patch branch March 7, 2024 01:17
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.

3 participants