alexforencich / verilog-axi

Verilog AXI components for FPGA implementation
MIT License
1.52k stars 454 forks source link

fix vivado synthesis problems in axil_reg_if_wr.v #39

Closed jameyhicks closed 2 years ago

alexforencich commented 2 years ago

What problem is this change supposed to solve, exactly?

jameyhicks commented 2 years ago

vivado complained about multiple drivers when there were registers assigned before the if (rst) and inside the conditions.

alexforencich commented 2 years ago

That doesn't make any sense, you only get "multiple drivers" when the signals are driven from multiple always blocks, multiple assign statements, etc. Not from within the same block. And just about every file in this repo is written in this style, so if you're only getting the warnings for this module, then that implies the problem is likely elsewhere in your design. Maybe you have a module with input/output mixed up, or some other incorrect connection?

jameyhicks commented 2 years ago

I was actually trying to make a PR against zhizhenzhong/master but I can improve the comments and clean it up.

alexforencich commented 2 years ago

You can make a PR there if you want. My point is that, at least so far, this change seems entirely unnecessary. The code builds fine on all current targets supported by Corundum with no multiple driver issues. And it makes zero sense that you would see an issue on the write side but not on the read side, since both modules are implemented in exactly the same way (or really with any other module in this repo, since most of them use this style to remove unnecessary resets and reset-driven-clock-enables). So I suspect that the multiple driver warnings that you're getting may actually be coming from somewhere else in your design, and not from this module.

jameyhicks commented 2 years ago

You are right. This makes no sense. Looking again, I found that the problem was in another part of the design. I don't know why this change seemed to fix things last week.

alexforencich commented 2 years ago

Nice, glad you found the actual problem!