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

Doc fixes and add comments to axi4 bundles #2925

Merged
merged 2 commits into from
May 13, 2022

Conversation

jiegec
Copy link
Contributor

@jiegec jiegec commented Jan 5, 2022

Related issue: None

Type of change: other enhancement

Impact: no functional change

Development Phase: implementation

Release Notes

  • fixed a parameter typo where privileged was misspelled within AHPParameters and APBParameters.
    • PROT_PRIVILEDGED is now PROT_PRIVILEGED

@sequencer
Copy link
Member

IMHO, porting like this makes RC more friendly to newbies, most of them never reads Chisel2 codes before. And Chisel also leaks of compatibility layer docs.
I’d like to see this happen if there is no objection from @aswaterman

@aswaterman
Copy link
Member

I’m ok with it if and only if some representatives designs are proven equivalent.

@jerryz123
Copy link
Contributor

Splitting this up into separate PRs to add documentation/upgrade to chisel3 is the way to go.

@jiegec jiegec changed the title Doc fixes and migrate from Chisel to chisel3 Doc fixes and add comments to axi4 bundles May 12, 2022
Copy link
Contributor

@michael-etzkorn michael-etzkorn left a comment

Choose a reason for hiding this comment

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

Now that the controversial chisel3 changes are moved to here #2982

I think these NFC are worth merging. They're in line with the direction of better documentation #2960

@sequencer sequencer dismissed michael-etzkorn’s stale review May 12, 2022 19:41

Don't merge since leaks sufficient tests.

src/main/scala/amba/axi4/AsyncCrossing.scala Outdated Show resolved Hide resolved
src/main/scala/amba/axi4/AsyncCrossing.scala Outdated Show resolved Hide resolved
src/main/scala/amba/axi4/AsyncCrossing.scala Outdated Show resolved Hide resolved
src/main/scala/amba/axi4/AsyncCrossing.scala Outdated Show resolved Hide resolved
src/main/scala/amba/ahb/Protocol.scala Show resolved Hide resolved
src/main/scala/amba/apb/Protocol.scala Show resolved Hide resolved
@sequencer
Copy link
Member

Sorry, @michael-etzkorn I just found the author has rebased the PR, but there I do have some nitpick on this, since it needs a deprecation notice and type/function alias for the typo fix.

@jiegec jiegec force-pushed the doc-fixes branch 5 times, most recently from 886e645 to da516dd Compare May 13, 2022 06:19
@jiegec jiegec force-pushed the doc-fixes branch 2 times, most recently from bd85a91 to d9f5176 Compare May 13, 2022 06:36
@sequencer sequencer enabled auto-merge (squash) May 13, 2022 07:16
@sequencer sequencer merged commit 6c762f3 into chipsalliance:master May 13, 2022
sequencer added a commit to sequencer/fpga-shells that referenced this pull request May 13, 2022
sequencer added a commit to chipsalliance/rocket-chip-fpga-shells that referenced this pull request Jun 5, 2022
jerryz123 added a commit to chipsalliance/rocket-chip-fpga-shells that referenced this pull request Sep 2, 2022
@jiegec jiegec deleted the doc-fixes branch November 27, 2024 03:35
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.

5 participants