|
发表于 2007-9-18 17:08:20
|
显示全部楼层
如果真是楼主写的代码, 那就值得商榷了.
1. 既然是同步fifo, 为什么是读写两个时钟呢? 笔误? 但问题是异步fifo又不能采用这种设计方法, 而且你的data_count/full/empty全部都是读时钟域的, 所以从这一点上看, 这个代码得分为0分.
2. always@(posedge clkw, web, reset) 大量类似的行, 问题很多啊.
(1)敏感量列表中各信号以逗号间隔, 这样的写法兼容性不好, 因为Verilog1995不支持, 2001才支持.
(2)always后面同时出现边沿敏感和电平敏感, 仿真没有问题, 但综合会出错, 或得不到期望的电路. 严重的说, 这是根本没有掌握RTL设计技能的表现, 不知道楼主是否正确理解同步复位和异步复位, 而且web不是复位信号, 不能写在敏感量列表中.
3. integer write_addr,read_addr; 这不是正确的RTL设计.
我知道楼主对数据位宽和fifo深度都采用了参数化, 所以不好确定读写地址的位宽, 我一贯采用的方法是, parameter AW为地址位宽, DW为数据位宽, 那么再定义一个FD=1<<AW, 表示fifo深度.
4. input [DATA_SIZE-1:0] din;
wr_data <= 8'h00;
既然已经参数化了, 那么wr_data就不能复位成8'h00, 如果该模块在被实例化时参数DATA_SIZE被重定义为9呢? 那么wr_data复位值的正确写法是什么呢, 这个问题就留给楼主思考一下, 而且如果复位值是DATA_SIZE位宽的全1呢? 如果复位值是DATA_SIZE位宽的"MSB和LSB都是1, 中间都是0"呢?
5. rd_data是垃圾代码, 不知道楼主写完后有没有仔细检查一下.
6. if (write_addr < (FIFO_DEPTH -1))
write_addr <= write_addr + 1;
else
write_addr <= 0;
这个没有必要, 直接write_addr <= write_addr + 1;就可以了, 到全1后会自动变为0, 如果明确定义了write_addr/read_addr位宽的话.
7. 读写操作完全由外面输入的web/reb来控制, 没有做安全防范处理, 健壮性太差. 如果full有效后web仍然有效呢? 而且就算外面的模块都对, 他看到full以后, 也会有一个延迟(通常是1个cycle), 才会撤销web, 那么在这个延迟的时间内, full和web都有效, 怎么办?
8. data_count实时记录了fifo数据深度, 但full和empty是触发器输出, 会晚一拍, 这会不会有问题? 能不能做到同步变化(data_count变为0, 则empty也同时变为1), 而且full和empty仍然是触发器输出呢? 好好考虑一下.
9. 其他一些代码风格.
建议每个always都有begin...end, 视觉效果好一些.
建议@前面, if后面, ==两边, 不要有空格.
建议在非阻塞赋值后面加上单位延时: a <= #1 b;
建议begin不要换行, 这样看起来更紧凑一点;
建议使用下面的注释来区分不同的功能块(比如写时钟域的逻辑一块, 读时钟域的逻辑一块):
//=======================================
//
//=======================================
建议内部逻辑信号, 在功能描述的地方再定义, 而不是全部都在in/out描述后面就定义了, 尤其对于大模块, 可读性好一些;
建议使用缩进对齐, 我都是使用tab缩进, tabstop设置为4.
[ 本帖最后由 fenzaiqi 于 2007-9-19 10:43 编辑 ] |
|