Open rroohhh opened 4 years ago
This is related to the discussion in https://github.com/nmigen/nmigen-boards/pull/75.
TL;DR The DSL needs to be redesigned. @whitequark doesn't have time right now, I have offered to help, she will probably ping me when she has time.
I don't think this choice depends in any way on the redesigned DSL.
What about the DE10-Nano and MiSTer situation? The latter is basically a board mounted on the former, and so pins which are free connectors on the DE10 are instead Resources for the MiSTer. Is that an instance of 2, or its own special case?
I'm not actually sure. I'd say special case, there wouldn't be that many of those.
@Ravenslofty good question, I think making the MiSTer inherit from the DE10-Nano and add its own Resources (like it is done here) would work.
Summarizing what I'm doing with the ULX3S, which theoretically has a variant for each FPGA type:
device
as not to be instantiable) with the shared board definitions.argparse
to select which variant should be used for the test code, like so:if __name__ == "__main__":
from .test.blinky import *
variants = {
'12F': ULX3S_12F_Platform,
'25F': ULX3S_25F_Platform,
'45F': ULX3S_45F_Platform,
'85F': ULX3S_85F_Platform
}
# Figure out which FPGA variant we want to target...
parser = argparse.ArgumentParser()
parser.add_argument('variant', choices=variants.keys())
args = parser.parse_args()
# ... and run Blinky on it.
platform = variants[args.variant]
platform().build(Blinky(), do_program=True)
I'd summarize my suggestions as follows:
One remaining question is whether "revisions" of a given board (e.g. OrangeCrab r0.1/r0.2) should be in separate files. I don't think this entirely matters, but I think that I'd default to treating them like variants (same file) if they're more or less the same hardware, and as separate platforms (different files) if the hardware's majorly changed. Example: If r0.2 board moves some pins around and changes an ADC, it'd be similar to a variant, and could hang around in the same file. If an r0.2 adds several new connectors, it'd be essentially a separate platform deserving of its own file.
Thoughts?
@ktemkin I like your suggestions, do you have a good idea on how to handle different speedgrades? I feel like adding a variant for each combination of speedgrade and size would bloat the number of boards quite alot, but that might be acceptable, as not that many boards seems to be available with different speedgrades.
I like your suggestions, do you have a good idea on how to handle different speedgrades
I'd consider treating them like any other variant (e.g. with FPGA size), and potentially changing our strategy only if we run into situations where we find boards with a wide variety of speed grades and sizes.
A couple of mitigating factors for that:
I see several factors that can guide our decisions:
If downstream code is highly likely to need programmatic choice between board variants, then we should think about making those constructor arguments rather than separate board classes. But if a typical project would only care about one variant, then separate classes give a bit more clarity, since there are fewer concepts to keep in mind (different board = different class, no need to look if it has a constructor instead).
Most of my projects are developed on a dev board and on "real" hardware simultaneously, which means I need programmatic choice between boards, but not necessarily board variants, FWIW. If I do a new revision I'm likely to switch to it entirely rather than supporting old boards. But my projects are not, themselves, dev boards, so someone like Greg or Luke may feel differently.
I found another interesting case with the MicroZed (#99). Is is available in two variants (with Zynq 7010 and 7020) and those differ in which pins are available on the two connectors it has. (As the Zynq 7020 has one IO Bank more than the 7010). There are no other resources on it.
Sharing part of the connector definition between the two boards would be nice, however it is not quite clear how this should be done. The only way I can think of right now is putting the connector pins definition in a string template and then removing the pins not available using string processing in the 7010 variant, like this:
import re
connector_template = "F9 J6 "
"F6 G6 "
"- - "
"- R11 "
"R19 T19 "
"T11 T12 "
"T10 U12 "
"- - "
"B13_U13 B13_V12 "
"B13_V13 B13_W13 "
"- - "
"T14 P14 "
"T15 R14 "
class MicroZedZ010Platform(Xilinx7SeriesPlatform):
...
connectors = [
Connector("JX1", 0, re.sub(r'B13_[^\s]*', "- ", connector_template))
]
class MicroZedZ020Platform(Xilinx7SeriesPlatform):
....
connectors = [
Connector("JX1", 0, connector_template.replace("B13_", ""))
]
One downside of this approach is, that it is not as obvious as before (just writing the connector definition twice) what is happening.
There seem to be two subclasses of very similar boards:
tinyfpga_ax{1,2}
and thezturn_lite_z0{07s,10}
. Currently these are handled as different boards, located in different files. Thezturn_lite
uses inheritance to avoid duplicating the pinout.ecpix
).The approach of making every variant its own board currently seems to bring two main advantages:
zturn_lite
) is completely seperate from each other, making maintance easier (changing a variant only ever affects that variant).Considering these advantages the approach for the second class seems very reasonable already.
For the first class however it might be better to put them in the same file and use inheritance like the
zturn_lite
to avoid duplicating pinout, programming logic and more.Especially for speedgrade however this still seems a bit unnecessary and could bloat the list boards.