lowRISC / opentitan

OpenTitan: Open source silicon root of trust
https://www.opentitan.org
Apache License 2.0
2.61k stars 787 forks source link

SPI programming on NexysVideo FPGA seems broken on some FPGA builds #3606

Closed mdhayter closed 4 years ago

mdhayter commented 4 years ago

This has happened to me twice now (in maybe 6 FPGA builds).

Some builds of the NexysVideo FPGA seem to fail to allow spi programming.

The Bootloader seems to fail to correctly receive/ack block 0 and either stops after printing "Processing frame #0, expecting #0" or it just loops with that message and spiflash reports bad ack.

In both cases I saw this immediately after building and loading an FPGA bitstream. Reloading the bitstream did not help, but rebuilding and reloading it fixed the problem (no changes to verilog and no rebuild of the software so the only change is whatever randomness is in the P&R).

Maybe we have a timing requirement on the SPI I/Os that isn't captured correctly in the Xilinx control files?

Note: At this point I have not looked through all the Xilinx logs to see if there is a hidden warning.

tjaychen commented 4 years ago

@imphil @msfschaffner

hey Mark, I think Michael also observed similar instabilities. Although in his case I think retrying some time helped? I don't recall us specifying any timing on the SPI pins at all, so it makes sense there could be some issues.

However I think in the most recent build we should have slowed the FTDI frequency down into the KHz range, and since it's a half cycle protocol I would not have expected hold time issues either, so I was hoping it would just work. It's possible we have other issues in the design though. I can help add some input / output delay constraints next week and see if it helps.

vogelpi commented 4 years ago

Thanks @mdhayter for reporting this issue. I can confirm the same behavior on the ChipWhisperer CW305 FPGA board.

vogelpi commented 4 years ago

I had a look and noted that there were critical warnings about a clock constraint not being applicable. The affected clock is the div2 clock which seems unrelated to SPI. However, as a result of the missing clock constraint, we had excessively long implementation times and still ended up having around -10ns WNS. I have now created a PR in #3625 to fix this. Let's see if that helps.

vogelpi commented 4 years ago

Update: as part of #3647 I have made the following changes to SPI:

I've tested this change with different bitstreams (generated from the same sources) and it was working reliably. Thus, I am closing this issue now.

Note, when doing this work, I also had a look at the device-side bootstrap code and think we should rework that. This is tracked in #3682 .