search

m-labs / misoc

The original high performance and small footprint system-on-chip based on Migen™
https://m-labs.hk
Other
306 stars 86 forks source link

spi2: add iCE40 differential interface #98

Closed airwoodix closed 5 years ago

airwoodix commented 5 years ago

This patch adds support for SPI-over-LVDS for iCE40 platforms.

The logic is taken straight from the 7-Series differential interface, which produces a lot of code duplication. Refactoring would be great. Would for example differential tristate buffers as generic Specials be a good idea?

The miso buffer suffers the same problems as DifferentialInput discussed in m-labs/migen#178 with yosys/nextpnr getting confused if the p side of the differential pair is in the pcf.

Reviews most welcome. Thanks in advance!

sbourdeauducq commented 5 years ago

Would for example differential tristate buffers as generic Specials be a good idea?

Since we already have DifferentialInput and DifferentialOutput, it makes sense. Would you mind adding it and then refactoring the old SPI code?

jordens commented 5 years ago

That won't work. Tristate differentials can't be represented in the ice40 primitive. Whether that's a silicon limitation is unclear.

airwoodix commented 5 years ago

@jordens: do you mean the full abstraction? The interface as submitted works for the ice40-hx8k-ct256 of Humpback to interface a Urukul EEM. It samples miso bit-reversedinverted though (but it's not yet clear to me why). This functionality is IMHO anyhow needed to get Humpback operational. Or did I miss something?

jordens commented 5 years ago

I meant just the idea of a differential tri-state for ice 40.

Your code is fine. Just add a note that it doesn't support half duplex/three wire.

airwoodix commented 5 years ago

Ok, thanks for the feedback. So no refactoring. I added some documentation and removed half duplex support more explicitly. The SB_IO cell in differential input mode samples inverted (see m-labs/migen#181), which also affects miso sampling. This is fixed by 1d70b45.

airwoodix commented 5 years ago

The discussion in m-labs/migen#181 also affects this PR. 1f2b5a2 fixes the polarity of the differential input for miso the same way it's done in m-labs/migen#182. Since the EEM connectors of Humpback have reversed polarity (see sinara-hw/Humpback#9), this will require being careful when instantiating the SPI interface and manually inverting the miso data.

airwoodix commented 5 years ago

@jordens is there a reason not to merge this? Do you have a better implementation planned for Banker/Humpback?

jordens commented 5 years ago

I just forgot. Sorry. For banker I have an SPI slave and register bank interface that doesn't need an external clock. It's rather entangled with the rest of the code and I had the impression that extracting it might be harder than just duplicating it.

airwoodix commented 5 years ago

Great, thanks! No problem! A generic SPI slave core would be great for misoc but it indeed doesn't interfere with this patch.