Hi,

Le Mon, 20 Sep 2010 23:49:20 +0200,
Michael Walle <[email protected]> a écrit :
> attached are two patches. one adds the bram core with the rom in it.

Comments in no particular order:
+ Use a synchronous reset.
+ Use the same Verilog coding convention as in the rest of the code:
  indent with one tab (width 8), no space after if/case/..., place
  "begin" on the same line as "if" and "always".
+ Include lm32_include.v in system.v (for CFG_ROM_DEBUG_ENABLED)
+ The RAMB16BWER primitive on Spartan-6 has byte-wide write enables, so
  we should try to use them instead of the read-modify-write state
  machine (speaking of which, the states s_idle and s_ack should be
  merged). I hope Xst can infer them e.g. by using statements like
  mem[adr][7:0] <= ...
+ Xst will find the ROM initialization file if it is placed in the same
  directory as the Verilog file calling $readmemh. So I added a
  pre-compiled .rom file in the "rtl" directory and moved the source to
  software/ so the .rom file can be manually reconstructed if needed -
  since I the debug ROM contents won't be modified often I think this
  solution is acceptable and doesn't justify a more complex build
  system...
+ Specify the width of bit constants (for example: ack <= 1'b0 instead
  of ack <= 0). Verilog assumes a default width of 32 for constants. It
  seems Xst handles this correctly and strips off the extra bits, but
  I have read in several places that synthesizable code should always
  specify bit widths, and it also avoids a pitfall if your vector has
  more than 32 bits.

I have addressed these issues (except the write enable one) in this
commit:
http://github.com/lekernel/milkymist/commit/73c6bd9f17cf3ab8a144d518d7ded72262c00c25

> the second adds the rcsr and wcsr command to the bios.

Merged with no change.

> we need some (dynamically generated define, which points to the
> MM_TOP directory). ATM i hardcoded the path to the debug rom into
> system.v. That should be changed :)

See comment above.

> where do we put that jtag core? into the lm32 directory? into the
> cores dir? if not into the cores dir, where do we put the tests?

Yes, I think we should put it in the LM32 folder, and the tests in
cores/lm32/test.

Thanks for the patches!
S.
_______________________________________________
http://lists.milkymist.org/listinfo.cgi/devel-milkymist.org
IRC: #milkym...@freenode
Webchat: www.milkymist.org/irc.html
Wiki: www.milkymist.org/wiki

Reply via email to