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

Scratchpad Documentation Fix #569

Merged
merged 1 commit into from
May 26, 2020
Merged

Scratchpad Documentation Fix #569

merged 1 commit into from
May 26, 2020

Conversation

abejgonzalez
Copy link
Contributor

Related issue: #567

Type of change: bug fix

Impact: other

Release Notes
Fixes the docs.

@jerryz123
Copy link
Contributor

I'm actually in favor of not documenting or including the DTIM scratchpad option. It doesn't seem to be something we would ever use.

@alonamid
Copy link
Contributor

Is there a reason not to document it?

@jerryz123
Copy link
Contributor

It's not our job to document all of rocketchip, especially the features which we don't regularly use.

@abejgonzalez
Copy link
Contributor Author

It sounds like external users like both options described (2 external people looked at the documentation and asked questions / gave feedback on it). So I think its beneficial to keep it.

@alonamid
Copy link
Contributor

To the actual content of the PR - there seems to be a mismatch between the text and the code example. You remove the memory port in the L1 code example, but in the text you say that you should do that for the mbus scratchpad case.

@abejgonzalez
Copy link
Contributor Author

No I do that for both cases?

@alonamid
Copy link
Contributor

you're right, github diff got me again...

Copy link
Contributor

@alonamid alonamid left a comment

Choose a reason for hiding this comment

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

LGTM

@abejgonzalez abejgonzalez merged commit 7ee4cad into dev May 26, 2020
@abejgonzalez abejgonzalez deleted the doc-fix branch May 26, 2020 19:58
@alonamid alonamid mentioned this pull request May 30, 2020
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