Skip to content

V
VME64x core
Commit c1fdaebc authored by tstana's avatar tstana
Browse files
Options

Thedi: added my Tuesday update. 

git-svn-id: http://svn.ohwr.org/vme64x-core/trunk@183 665b4545-5c6b-4c24-801b-41150b02b44b
parent 982176d0
Hide whitespace changes
Inline Side-by-side
Showing with 29 additions and 8 deletions
hdl/vme64x-core/rtl/CodeReview_2Nov2012/thedi.txt
View file @ c1fdaebc
......@@ -2,11 +2,12 @@ TODO (for me):
- check comments vs. user manual before final commit
- VMEAccessDecode: check specifications to see if WB process needs reset
- check for VHDL comments and coding guidelines
- synthesize design and check for latches in VMEAmMatch.vhd
x synthesize design and check for latches in VMEAmMatch.vhd
- check the [!!!] statements
General stuff
--------------------------------------------------------------------------------
Really like the fact that you wrote a lot of comments in the file headers. These
I really like the fact that you wrote a lot of comments in the file headers. These
are very descriptive and very easy to read, which helps users quickly get up to
speed on how they can use your core in their design.
......@@ -14,8 +15,15 @@ It might help to have some comments accompanying your entity ports. This can
help users quickly instantiate your core (or components of your core), in case
they need them.
Another fairly bothersome thing is the fact that your ISE project is not portable.
When I tried to synthesize your design, it was asking me for locating the
design sources. Which brings me to another not-very-good thing. You might want
to rethink file and folder hierarchy here-and-there. For example, the
VME_Package.vhd file is located in
[...]/trunk/hdl/boards/svec/sim/testbench/VME64x_Package.vhd
This file seems to be very important in your design. Thus, it should be located
somewhere more logical from a user perspective, e.g., the hdl/vme64x/ folder.
You have some more detailed comments below.
--------------------------------------------------------------------------------
......@@ -87,9 +95,22 @@ don't make too much sense.
- line 14: typo
"[...] different address wide [...]" -> "[...] different address widths [...]"
- p_AMmatch:
Are you sure the synthesis tool is not inferring latches here? You have a pretty
large amount of conditions uncovered, is it capable of generating registers with
enable from that? I will try to synthesize your design to see if it can...
Expect an answer on this shortly. :)
- last two processes:
I find the first statements in these processes redundant. Do they help any by
being there?
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
ram_8bits.vhd
spram.vhd
xwb_ram.vhd
--------------------------------------------------------------------------------
There is an undefined constant here -- c_SIZE, that you are using to initialize
your g_size generic. However, this constant is not defined anywhere in the context
of these files. I tried finding it in the VME_Package.vhd file as well, but it's
not defined.
You should either use predefined values for these generics, or defined this
constant somewhere in a package and "use" it in your design sources. I would
recommend the former.
--------------------------------------------------------------------------------
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment