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

cli: add flash-fast command #25

Merged
merged 3 commits into from
Mar 27, 2024
Merged

Conversation

mndza
Copy link
Contributor

@mndza mndza commented Sep 8, 2023

This PR adds a command to Apollo CLI (apollo flash-fast) for flash programming. This command first programs a gateware SPI bridge to the FPGA and then uses it to access the configuration SPI flash.

@mndza mndza marked this pull request as ready for review October 9, 2023 11:00
Copy link
Member

@martinling martinling left a comment

Choose a reason for hiding this comment

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

I'm not sure if it makes sense to review this while greatscottgadgets/luna#220 is still in draft, since the two go together.

In fact, perhaps that flash bridge gateware should live here in the Apollo repository rather than in the LUNA repo?

@mndza
Copy link
Contributor Author

mndza commented Oct 9, 2023

True, let's wait until that other PR is ready (I'm marking as draft to avoid confusion).

I would like to create a PR with the ApolloAdvertiser injection code first (should that go in the cynthion repo?). That's why greatscottgadgets/luna#220 is still a draft (it creates a manual instance of the advertiser).

I do not have a strong opinion on the location of the flash bridge gateware. If we decide the code belongs here, I'm willing to move it.

@mndza mndza marked this pull request as draft October 9, 2023 13:28
@martinling
Copy link
Member

There's three parts as I see it:

  • support for platform-specific hooks, which needs to be added to USBDevice in the LUNA repo.
  • the hooks, which need to be added to the platform definitions in the Cynthion repo.
  • the ApolloAdvertiser gateware, which I think should live in apollo_fpga.gateware and can be imported from there by all Apollo-based platforms that need it (which will include Cynthion, URTI and others).

@mndza mndza marked this pull request as ready for review October 11, 2023 14:36
@mndza mndza marked this pull request as draft November 2, 2023 15:58
@mndza mndza marked this pull request as ready for review March 13, 2024 10:36
@mndza mndza requested a review from martinling March 13, 2024 10:36
@mndza
Copy link
Contributor Author

mndza commented Mar 13, 2024

Rebased.

  • Changed bInterfaceSubclass to 0x01 to avoid conflict with Apollo stub interface.
  • Added a final trigger reconfiguration step when closing the connection with the flash bridge. This makes flash-fast inconsistent with the flash command (which does not trigger a reconfiguration), but avoids keeping the Flash bridge enumerated.

Copy link
Member

@martinling martinling left a comment

Choose a reason for hiding this comment

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

I've tested this on cynthion-test and it's all working great, but I have a couple of comments where we may need to discuss what we want to do.

apollo_fpga/commands/cli.py Outdated Show resolved Hide resolved
apollo_fpga/gateware/flash_bridge.py Outdated Show resolved Hide resolved
mndza added 3 commits March 27, 2024 14:23
Copy link
Member

@martinling martinling left a comment

Choose a reason for hiding this comment

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

All good!

@martinling martinling merged commit d00d0d5 into greatscottgadgets:master Mar 27, 2024
3 checks passed
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.

2 participants