-
Notifications
You must be signed in to change notification settings - Fork 175
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
[RFC] Add InstanceResource to use Instance I/O as Platform I/O #525
Comments
Okay, this isn't really an RFC because this doesn't explain the exact mechanism through which this will happen. There isn't anything to comment on. |
I propose, that we just allow to pass and Usage Example for the proposal: class ExamplePlatform(LatticeMachXO2Platform):
device = "LCMXO2-2000HC"
package = "TG100"
speed = "6"
default_clk = "clk_internal"
class Osc(Elaboratable):
def __init__(self, freq=2.08e6):
self.freq = freq
self.i = Signal()
def elaborate(self, platform):
m = Module()
inst = m.submodules.inst = Instance("OSCH", o_OSC=self.i)
inst.attrs["NOM_FREQ"] = str(self.freq / 1e6)
return m
resources = [
Resource("clk_internal", 0, Osc())
] |
I like this direction. I think wrapping There are multiple unresolved questions:
|
Here is my counter-RFC. MechanismWe currently have a bunch of methods like: def get_input_output(self, pin, port, attrs, invert): Let's add two more, which connect IO of a requested resource to a particular port of a particular internal component. Both the def get_internal_signal(self, signal, name, port, attrs, invert):
def get_internal_pin(self, pin, name, port, attrs, invert): There would be two behaviors added to the DSL. First, there would be a new class Internal:
def __init__(self, name, port, *, dir, width=1, invert=False): When requested, this Second, the existing When requested, this Like other similar methods, ExamplesUsing ECP5 SPI Flash would look like: *SPIFlashResources(0,
cs="R2", clk="@USRMCLK.MCLK", cipo="V2", copi="W2", wp="Y2", hold="W1",
attrs=Attrs(IO_TYPE="LVCMOS33")
), Using Cyclone-V HPS might look like: Resource("rst", 0, InternalN("hps_system", "hps_fpga_reset_reset_n", dir="i")),
Resource("clk50", 0, Internal("hps_system", "clk_50_clk", dir="i"), Clock(50e6)),
Resource("ddr3", 0,
Subsignal("rst", PinsN("@hps_system.memory_mem_reset_n", dir="o")),
Subsignal("clk", DiffPairs("@hps_system.memory_mem_reset",
"@hps_system.memory_mem_reset_n", dir="o")),
Subsignal("clk_en", Pins("@hps_system.memory_mem_cke", dir="o")),
Subsignal("cs", PinsN("@hps_system.memory_mem_cs_n", dir="o")),
Subsignal("we", PinsN("@hps_system.memory_mem_we_n", dir="o")),
Subsignal("ras", PinsN("@hps_system.memory_mem_ras_n", dir="o")),
Subsignal("cas", PinsN("@hps_system.memory_mem_cas_n", dir="o")), I think Cyclone-V HPS looks like it might require the use of BenefitsThis mechanism has many desirable properties:
In particular, this would satisfy:
It would also let us get rid of the weird custom code that instantiates DrawbacksWhile this mechanism is fairly simple to implement, it may be hard to understand because of the extra overloading and indirection. This particularly applies to the This mechanism adds two ways to do almost the same thing: Alternatives
|
How would passing arguments / config to the internal/hard block work ? Because those config would/could be "global" and not per-signal/pin. |
Depends. Some hard blocks are singletons, but it is actually (counterintuitively) better to instantiate them as many times as requested. E.g. suppose we are exposing For the rest I think it is appropriate to configure it out of band. E.g. your USB case. I imagine most designs will have 1 and only 1 USB gadget in them--so you can just add an attribute to the platform. But if you have more than one, then I can imagime some sort of method like |
One thing that the RFC doesn't mention (but might be obvious) is, how the With the connector system there is already one way of indirection, I was wondering if there is a way to maybe expand it to use it for internal connections aswell. |
Excellent question. I have actually spent a good while thinking about it and haven't decided anything. Chiefly, the issue is that a normal physical pin name always refers to exactly 1 pin, but the The closest thing to a solution I see is a compromise where |
This proposal, or my counter-proposal, would have to go through our new RFC process to be accepted. In general, the current system of defining pins should probably be scrapped entirely and replaced with something better, so there is little point in defining things on top of it. |
This is an RFC for #308.
Some chips like the Intel Cyclone V SoC and Xilinx Zynq require
Instance
s to access certain I/O functions not directly available through top-level module ports. Others, such as the Lattice iCE40, useInstance
s to source clocks.Presently, the latter chip manually instantiates
SB_HFOSC
, but I think there should be a better way.nMigen should provide an
InstanceResource
(or perhaps anInstanceConnector
?) to treat I/O from anInstance
as global I/O provided byplatform.request
.The text was updated successfully, but these errors were encountered: