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

Allow optional soft reset option to be passed into deassert_risc_reset_at_core #292

Merged
merged 2 commits into from
Nov 13, 2024

Conversation

abhullar-tt
Copy link
Contributor

@abhullar-tt abhullar-tt commented Nov 11, 2024

Issue

#287

Description

Metal needed ability to deassert only the second risc on BH ethernet cores

List of the changes

Added an optional parameter to deassert_risc_reset_at_core API

Testing

Will be opening up a PR in Metal on top of this branch and will run post commit and BH post commit.
Metal post commit
Metal Blackhole post commit

API Changes

This PR has non-breaking API changes:

@abhullar-tt abhullar-tt force-pushed the abhullar/deassert-risc-reset branch 2 times, most recently from 4de9cd9 to 0696c66 Compare November 11, 2024 17:26
Copy link
Contributor

@broskoTT broskoTT left a comment

Choose a reason for hiding this comment

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

Regarding comment in the PR:
This PR has API changes, but due to default value, no breaking API changes.
No need for PR links to any repo.

Any idea how could we maybe unit test this behavior in UMD?
Maybe at least add a very simple example to api_chip_test (we're trying to add any api to tests/api which we touch, and end up with all the APIs being touched).

@abhullar-tt
Copy link
Contributor Author

ple example to api_chip_test (we're trying to add any api to tests/api which we touch, and end up with all the APIs being touched).

we're

How about:

  • test that asserts reset, deasserts with legal value, then reads back reset reg to make sure the deassert goes through and
  • test that asserts reset, deasserts with some illegal value, then reads back reset reg to make sure its in a legal state?

@broskoTT broskoTT added the changes api API changing PR, needs changes in client code label Nov 13, 2024
@broskoTT
Copy link
Contributor

@abhullar-tt Thanks for the tests! Just take a look on why aren't they passing on remote chips...

@abhullar-tt abhullar-tt merged commit 049a51a into main Nov 13, 2024
19 checks passed
@abhullar-tt abhullar-tt deleted the abhullar/deassert-risc-reset branch November 13, 2024 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changes api API changing PR, needs changes in client code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants