Option to factor out "clk" from "spi_flash" on Lattice ECP5 #38
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ghost
mentioned this pull request
|
Shouldn't nmigen-boards (or nmigen) instantiate USRMCLK to deal with this case transparently for the user? |
|
Note that sometimes such primitives combine many unrelated signals (and you can always count on Xilinx for doing that, see |
* A new class `VersaECP5Platform_USRMCLK` appends SPIFlashResources that exclude a clk Subsignal
ghost
This was referenced Jan 27, 2020
| ) | ||
|
|
||
|
|
||
| class VersaECP5Platform_USRMCLK(_VersaECP5PlatformBase): |
There was a problem hiding this comment.
Probably better to add a parameter to __init__ instead of having two separate classes. This is what we did in other places (e.g. Migen), having one class for each combination of options does not scale.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
According to Section 6.1.2 of an official ECP5 manual, only the on-chip oscillator MCLK is available as the SPI clock when in Master SPI mode by default. To use any user clock as the SPI clock, a USRMCLK Instance must be instantiated and there is no need to request for the clock from Ball U3. Requesting for such clock will lead to an error as reported by @xobs on the nextpnr repo.
Therefore, I would like to suggest modification on memory.py such that there is an option to add the SPI flash resources to a platform (not only ECP5) without the clock. Although this modification has been used to successfully build and flash a bitstream on an ECP5 board (the example was an SPI reader module on my nmigen-stdio fork), I cannot deny that there is room for improvement on my code for this pull request.
I look forward to seeing further comments, thanks.
See also: my pull request on nmigen-stdio for a SPI controller.