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

Improved Debug Module documentation #3036

Merged
merged 4 commits into from
Oct 2, 2022

Conversation

midnighter95
Copy link
Contributor

Related issue:
Documentation

Type of change: bug report | feature request | other enhancement
other enhancement

Impact: no functional change | API addition (no impact on existing code) | API modification
no functional change

Development Phase: proposal | implementation
implementation
Release Notes

Add more micro-architecture details in documentation for debug module.
Fix format and typo issues in documentation.

@midnighter95
Copy link
Contributor Author

Ask for review @sequencer

Copy link
Member

@sequencer sequencer left a comment

Choose a reason for hiding this comment

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

I think this need more architecture level documentations.

Copy link
Contributor

@CircuitCoder CircuitCoder left a comment

Choose a reason for hiding this comment

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

Some format / content suggestions.

* @param nAbstractDataWords Number of 32-bit words for Abstract Commands
* @param nProgamBufferWords Number of 32-bit words for Program Buffer
* @param hasBusMaster Whether or not a bus master should be included
* @param clockGate todo
Copy link
Contributor

Choose a reason for hiding this comment

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

clockGate is used in Periphery.scala, where the debug module is instantiated.

@@ -563,6 +608,7 @@ class TLDebugModuleOuter(device: Device)(implicit p: Parameters) extends LazyMod
// hartsel/hasel/hamask must also be used by the DebugModule state machine,
// so it is passed to Inner.

// sends debug interruption to Core when dmcs.haltreq is set,
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe merge with the comment block above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In fact, the comment block above has nothing to do with Interrupt mechanism. So I'd rather keep this line and move the block above to where it belongs.

Copy link
Member

@sequencer sequencer left a comment

Choose a reason for hiding this comment

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

better than not having this, merge it to wait next documentation improving. ;p

@sequencer sequencer merged commit 78c77ee into chipsalliance:master Oct 2, 2022
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.

3 participants