Hi Michael,
thanks for your patch. Looks good, if you have tested it and it works
it's almost good for merging.
There are two minor style/indentation problems (I_set = 1'b1; and I_r <=
I;).
More importantly, some combinatorial always blocks generate latches and
this is not OK. This happens when the coverage of case statements is not
exhaustive. For example, next_irq_ack is not assigned by this block when
irq=0, and this produces a latch in hardware:
always @(*) begin
casex(irq)
8'bxxxx_xxx1: next_irq_ack = 8'b0000_0001;
8'bxxxx_xx10: next_irq_ack = 8'b0000_0010;
8'bxxxx_x100: next_irq_ack = 8'b0000_0100;
8'bxxxx_1000: next_irq_ack = 8'b0000_1000;
8'bxxx1_0000: next_irq_ack = 8'b0001_0000;
8'bxx10_0000: next_irq_ack = 8'b0010_0000;
8'bx100_0000: next_irq_ack = 8'b0100_0000;
8'b1000_0000: next_irq_ack = 8'b1000_0000;
endcase
end
This can be solved e.g. by adding next_irq_ack = 0 at the beginning of
the block.
Note that the whole block can be optionally rewritten as (yes, this is
synthesizable):
integer i;
always @(*) begin
next_irq_ack = 0;
for(i=0;i<8;i=i+1) begin
if(irq[i]) begin
next_irq_ack = 1 << i;
break;
end
end
end
Same remarks for the PC_ex block.
Best regards,
Sébastien
_______________________________________________
http://lists.milkymist.org/listinfo.cgi/devel-milkymist.org
IRC: #milkymist@Freenode
