Does anyone know why the fix wasn't implemented, and what the consequence of the DIHOLD logic being commented out is? It's been this way for years so I'm guessing that the core is working fine without the fix. But is there is anything I should look out for? Or should I just apply the suggested fix?
Gosh, this does seem like a long time ago! I can only assume I was being cautious. As Arlet was already aware of the issue, I was hoping it would get addressed upstream by him in due course. Which I think he did a few days later
here.
I find RDY logic is quite hard to test comprehensively. Though I guess you could just randomly assert RDY (and provide incorrect data when RDY is de-asserted). The use of RDY in the Matchbox Co Processor is quite limited. I think it's dangerous to generalize too much from that use case. Arlet's fix looks better to me than just commenting out the code!
What's your use case for RDY?
Are you using the 6502 or 65C02 version?
Looking at Atlet's fix in more detail:
Code: Select all
always @(posedge clk )
if( RDY )
DIHOLD <= DI;
assign DIMUX = ~RDY ? DIHOLD : DI;
This bit of logic ensures the DIMUX signal has the most recent valid DI sample. The rest of the core then uses DIMUX in place of DI. This guarantees that decisions are never made based on invalid (RDY=0) values of DI.
I would suggest using that fragment in the 65c02 core.
By the way, Artlet later developed his own microcoded 65C02 implementation:
https://github.com/Arlet/verilog-65C02-microcode
There is also the AlanD core, which the one I now use in my Acorn FPGA projects, as it's cycle accurate (*):
https://github.com/hoglet67/AtomBusMon/ ... R65Cx2.vhd
(*) well almost - I recently discovered RTI takes 7 cycles rather than 6. There are a few more cases that came to light now we have a comprehensive cycle accuracy test suite, see
here for the gory details.