[oc] GPIO core

[Richard Herveille <richard@asics.ws>]

Hi Damjan,

I am verifying your core and was browsing through your code. I think I 
found a potentially serious problem.

When using the GPIOs as inputs, the user has the option between latching 
the inputs on either the wishbone-clock or the external-clock. Before I 
make my statement let me point out that this is a good idea.

Now here's where my complaint starts:

You are using the following piece of code to generate an intermediate clock 
signal:

assign latch_clk = rgpio_ctrl[`GPIO_RGPIO_CTRL_ECLK] ?
		ext_clk_pad_i ^ rgpio_ctrl[`GPIO_RGPIO_CTRL_NEC] : wb_clk_i;


In general it is very bad coding style to generate a clock using 
combinatorial logic, especially the main-system clock. I know it is just 
for a single register-block (from 1 to 32 registers) and I know you have to 
use logic to be able to trigger on the rising and falling edge of the 
external clock signal. But there are better ways of doing this, like having 
separate registers for the different clocks and edges and then choosing the 
appropriate one. Also the GPIO inputs are asynchronous (whether they are 
latched by the external or wishbone clock), you should probably add a 
second register stage (clocked at the wishbone clock) to reduce metastability.

Richard

--
To unsubscribe from cores mailing list please visit http://www.opencores.org/mailinglists.shtml