1. Home
  2. Field-Programmable Gate Arrays
  3. Comment on my code style

Comment on my code style

Greetings:

I am learning Verilog from the book "A Verilog Primer 2nd. ed." by J. Bhasker.

Here's my crack at a digital one shot. It works fine, or course, since I have already used it in real hardware. But I am interested in pointers on coding style or any other comments people might like to make:

Note: some of the delays and the initial statement are for making the functional simulation work, but I know they get ignored during synthesis.

---------------------------------------------------------- /* This module implements a digital one-shot with N-bit resolution. Ports: Trig_in rising edge causes output pulse to begin Out positive going output pulse appears here Clk_in user must supply a clock here Delay user must supply an N-bit value here for the number of

clock periods to delay

How it works:

An N-bit counter gets clocked all the time by the incoming clock signal. A D-flop latches the input trigger and also enables the counter. The output pulse is taken from the D-flop Q output. Thus the leading edge of the output pulse is synchronous with the trigger edge. But the trailing edge of the output will be synchronous with the clock.

At the end of a delay cycle, the D-flop Q output falls and resets the counter, preparing it to count again. An incoming trigger pulse sets the D-flop, causing Q to rise, releasing the CLR of the counter. When the count reaches the Delay value, the D-flop is reset. This causes the Q to fall again, clearing and holding the counter until the next trigger.

*/

module DelayNbit(Trig_in, Out, Clk_in, Delay); parameter NUM_BITS = 8; // This sets the default number of bits of

counter resolution input Trig_in, Clk_in; input [NUM_BITS-1:0] Delay; output Out; wire Trig_in, Clk_in; wire [NUM_BITS-1:0] Delay; reg Out;

wire Clr_ff; // flip-flop asynchronous clear input

reg [NUM_BITS-1:0] Q_c; // register for N-bit counter

/* BEGIN behavioral model of the D flip-flop with asynchronous clear, adapted from Xilinx :

*/ initial Out = 0;

always @ ( posedge Trig_in or posedge Clr_ff ) begin if ( Clr_ff == 1 ) #4 Out

Reply to
Chris Carlen
Loading thread data ...

Interesting...

You've coded two standard clocked "always" blocks - fine.

You've declared all your inputs as "wire". That's not necessary, but it's a good idea.

I have a few issues with the cosmetics of your code - in particular I don't like using /*...*/ block comments, because they don't nest and you can easily get yourself into trouble with them; but that's a matter of style choice.

You don't *need* the delays for functional simulation, because nonblocking ( or course, since I have already used it in real

hmmmm.... the design has some "interesting" non-synchronous features that don't sit comfortably in FPGAs - I guess you're targeting Xilinx, from your comments - it would be very interesting to know your reasons for being confident about its reliability. I *think* I understand that it's OK to remove the reset (!Out) on Q_c asynchronously, although it could easily be subverted by a sufficiently fast Clk_in. I also am nervous of the clearing of Out by a pulse which is immediately terminated by Out clearing...

Generally it's very dangerous to use the asynchronous resets of FPGA flip-flops for anything other than startup initialisation. Timing analysis of the reset paths into a FF is vexatious at best.

Finally, from an FPGA point of view, these timeout counters are usually more compact if you use the trigger to load the counter with the delay value, then count down to zero; equality comparison with a constant is much cheaper than equality comparison with a variable. However, that strategy would (I think) demand a fully synchronised design.

--
Jonathan Bromley, Consultant

DOULOS - Developing Design Know-how
VHDL, Verilog, SystemC, Perl, Tcl/Tk, Verification, Project Services

Doulos Ltd. Church Hatch, 22 Market Place, Ringwood, BH24 1AW, UK
Tel: +44 (0)1425 471223          mail:jonathan.bromley@doulos.com
Fax: +44 (0)1425 471573                Web: http://www.doulos.com

The contents of this message may contain personal views which 
are not the views of Doulos Ltd., unless specifically stated.
Reply to
Jonathan Bromley

[snip]

I have a personal preference to improve readability of my own code. I have trouble digesting code that's sprawled over several pages so I try to compress things vertically when I can without crowding the code too much.

When an always block has just one construct - an if/else for async set/reset, for instance - the begin/end is superfluous. Indentation keeps my visual clues telling me what belongs in one begin/end construct or stands alone as one statement. When if/else constructs have short assignments, I like to do those in-line if the code isn't crowded. I'd code the block above as:

always @ ( posedge Trig_in or posedge Clr_ff ) if ( Clr_ff == 1 ) #4 Out

Reply to
John_H

Ok.

Yes, I discovered this, and simply wanted to make it look nicer so put in the delays, also knowing the synthesis would simply whine then ignore them.

Maybe parameterizing is a good idea, because I was hoping to figure out a way to have the delays active dependent upon who was instantiating the module, which can be controlled by using "module instance parameter value assignment" or defparam.

But there is still the question about how to direct conditional compilation of statements in an instantiated module from the instantiating module. Ie., I'd like to have an `ifdef to conditionally compile for instance the initial statement which is supoerfluous to implementation, but not for simulation. Or I'd like to not have to edit the module to change the defines, but simply adjust them from the top level. I don't quite know how this works yet. I'll have to do some experimenting.

Good question. Actually I suppose I can't be certain when implementing in a PLD. I am using XPLA3 CPLDs at the moment. I am confident in the circuit implemented in discrete logic, of course, because one can depend on the propagation delays to be sure that the main flip-flop reset remains true until it is removed after propagating the counter reset through the flip-flop, counter, and comparator.

But there are indeed questions about what goes on in the CPLD. Perhaps it would only be 100% safe if I coded it structurally and gate level, and used "WYSIWYG" mode of the implementation options.

I hadn't thought about what could go wrong here with the clock being close to the reset. I got the basic circuit from Art of Electronics,

2nd. ed., page 523. Are you thinking it is possible for some of the flops in the counter to interpret a clock edge close to the release of reset as happening before the reset removal, but some of the other flops interpreting it as not happening before the reset removal? One can envision this happening due to propagation delays over physical wires. But perhaps in the case of starting from zero, this isn't an issue. I'd have to think more carefully about the guts of the counter to be sure about this.

Yeah, I addressed this above, but what do you think of my analysis? First tell me (since you are probably a more experienced logic designer in general) how comfortable you would be with the design in discrete logic? Then in a PLD (considering my thoughts above)?

Well that's probably safer. I am becomming very interested in digital delay generation and the question of how to reduce jitter. Of course, my design has no leading edge jitter, and has up to one clock period of trailing edge jitter. A fully synchronous design would do what? I suppose there would be synchronization jitter on the leading edge (the trigger edge), but then the time would be without jitter. There has to be jitter somewhere.

Ok, thanks for the input.

Good day!

--
____________________________________
Christopher R. Carlen
Principal Laser/Optical Technologist
Sandia National Laboratories CA USA
crcarle@sandia.gov
Reply to
Chris Carlen
[...]

`define is global to a Verilog compilation, and therefore cannot be controlled instance-by-instance. There's no way to conditionalise compilation instance-by-instance in standard Verilog, but if your tools support Verilog-2001 (not a done deal by any means) then you can use "generate...if" to conditionalise some code based on the value of a parameter, which of course can be pushed down into an instance from its parent.

[...]

If it's in AoE it *must* be good :-)

Exactly so. In general, it is unsafe for an asynchronous signal to influence more than one FF. The jargon is "input hazard".

This is indeed the case. If the counter is held at zero, then only the very first flip-flop can be affected by the first clock after reset is removed; so the timing of reset w.r.t. clock doesn't matter (except that you might get metastability on the LSB counter bit, but that's another story). However, if you are trying to clock the counter very fast, it's just conceivable that the skew on Reset between bit 0 and bit 1 of the counter is greater than one clock period; in this case, it's just possible that the counter could successfully increment from 0 to 1, but the next bit is still held in reset by the time the NEXT clock arrives - so the counter goes back to zero again. However, this is very unlikely because the synth tools will surely lay out the counter so that its FFs are rather close together (so that it can use the ripple carry chain) and therefore the reset distribution skew is likely to be quite small. So don't worry, it's just me being paranoid as usual.

I think it's OK. Out is held in a FF. I think the design is a reliable asynchronous state machine, but when I said "nervous" I meant it - these things are hard to analyze, and it's very easy to screw up by extrapolating from a working design to a slightly different one that fails horribly, usually just after shipment.

Sure. A fully synch design has -0/+1 "jitter" both at the start and at the end, I think. No escape. (It's not "jitter" in the usual sense, rather "quantization error", but I guess that depends on what you're trying to do with the resulting signal).

OTOH you should trawl through the last few months' comp.arch.fpga archives and take a look at what Peter Alfke and others have had to say about using polyphase clocks for very high resolution time measurements. The Xilinx DLLs make all sorts of interesting things possible. I haven't followed that discussion in detail, but it sounds like a lot of fun.

--
Jonathan Bromley, Consultant

DOULOS - Developing Design Know-how
VHDL, Verilog, SystemC, Perl, Tcl/Tk, Verification, Project Services

Doulos Ltd. Church Hatch, 22 Market Place, Ringwood, BH24 1AW, UK
Tel: +44 (0)1425 471223          mail:jonathan.bromley@doulos.com
Fax: +44 (0)1425 471573                Web: http://www.doulos.com

The contents of this message may contain personal views which 
are not the views of Doulos Ltd., unless specifically stated.
Reply to
Jonathan Bromley

I see. I will have to keep this in mind for future stuff.

Things should be Ok at my 10kHz clock rate ;-D

I noticed that discussion, and my reaction was "what in the heck are they talking about?" I think after getting some experience with logic at the CPLD level I will head towards FPGAs. I'd like to first complete a formal text or course on the matter.

Thanks for the input.

Good day!

--
_____________________
Christopher R. Carlen
crobc@earthlink.net
Suse 8.1 Linux 2.4.19
Reply to
Chris Carlen

ElectronDepot website is not affiliated with any of the manufacturers or service providers discussed here. All logos and trade names are the property of their respective owners.