Hi,
(Cc to list as the comments may be of interest to other potential
contributors)
On Wednesday 23 September 2009 16:17:42 you wrote:
> tmatsuya wants you to pull from tmatsuya/milkymist at master
>
> Body: added cores/ps2/rtl/simple_ps2.v
Thanks. I have a few concerns about your design:
- Please use synchronous reset (ie. line 26 should be "always @(posedge
sys_clk)")
- Please keep the design synchronous (in practice, use as few clock domains as
possible). Therefore, replace "always @(posedge counter[clk_range])" with
"always @(posedge sys_clk)" and use a clock enable (ie.
if(counter[clk_range])...) ). See the UART for an example in a similar case.
- To avoid metastability issues, please double-latch all asynchronous inputs
with respect to the system clock, i.e.
reg ps2_clk_1;
reg ps2_data_1;
reg ps2_clk_2;
reg ps2_data_2;
always @(posedge sys_clk) begin
ps2_clk_1 <= ps2_clk;
ps2_data_1 <= ps2_data;
ps2_clk_2 <= ps2_clk_1;
ps2_data_2 <= ps2_data_1;
end
and then, use ps2_clk_2 and ps2_data_2 only in the rest of the design. Yes,
it's paranoid, but sporadic failures are the bane of the HW designer.
- Please add a register on csr_do (limits timing problems when your core is
part of a large SoC)
- You may want to add a means for the the CPU to ack the IRQ.
- Add an header to the file including a copyright notice with your name.
If you have questions please let me know.
Regards,
Sébastien
_______________________________________________
http://lists.milkymist.org/listinfo.cgi/devel-milkymist.org
IRC: #milkym...@freenode
Webchat: www.milkymist.org/irc.html
Wiki: www.milkymist.org/wiki
