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

Add openocd option to RevB settings #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sctincman
Copy link

With riscv-collab/riscv-openocd#456, the RevB now has OpenOCD support. Add tag to settings.mk to allow use in freedom-e-sdk.

This also requires makefile changes to key off of, but this will need to be merged first I think before the freedom-e-sdk bsp CI tests will pass.

@nategraff-sifive
Copy link
Contributor

Thanks @sctincman! Can you go ahead and submit a PR to freedom-e-sdk with the changes you suggest there to support the HiFive1 RevB optionally using OpenOCD? When we have these cross-repo dependencies we usually use the following workflow:

  1. Submit a PR to the child repo
  2. Submit a PR to the parent repo, pointing at the child repo branch
  3. Review the changes together and show that the tests pass in both PRs
  4. Merge the child repo, rebase the parent PR to include child master with the change, and merge the parent repo.

Copy link

@sifivekevin sifivekevin left a comment

Choose a reason for hiding this comment

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

These changes look good on visual review. I cannot test because I lent my HiFive1 RevB target out and haven't received it back yet.

@sctincman
Copy link
Author

@nategraff-sifive, certainly! Will do. I was wondering how the e-sdk was handled with its large number of submodules.

Should I also update the submodule target using the child PR commit? I imagine that's what you mean will be rebased ontop of merged child?

Lastly, the makefile change I currently have is built on top of my other PR (sifive/freedom-e-sdk#441), but that seems to be in limbo at the moment. Could I potentially get that reviewed? Or should I make the change on top of what's in master at the moment.

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