VHDL coding style question

Status
Not open for further replies.

shaiko

Advanced Member level 5
Joined
Aug 20, 2011
Messages
2,644
Helped
303
Reputation
608
Reaction score
297
Trophy points
1,363
Visit site
Activity points
18,302
Hello,

This is a question about coding styles and your approach to designing digital circuits in HDL - I'll phrase as:

When you're describing reusable logic (not customized logic with special features):
How deep do you hierarchize your design?
Or, what are your design "atoms"?

A simple example to elaborate:

An SPI peripheral consists of several logical components:

Shift register, clock generation logic, IO toggling, flag generation(ready/received data), etc...
If you look deeper - the clock generation logic will always have a counter in its heart and a comparator.

1. Will you design the SPI peripheral as a single component?
2. Will you divide it to components (shift register, clock generator , etc...) in several files ?
3. Will you go even further down and describe the clock generator as a counter and comparator ?
4. Maybe even (getting crazy) deeper?
 

I would describe my design down to even gate level logic(so that I can get a gate estimate as well as get to know the number of logic levels) and flop level logic.
The decision on the number of files to be used will be based on the functionality I suppose..(you might want to group stuff performing a common function together)
 
Reactions: shaiko

    shaiko

    Points: 2
    Helpful Answer Positive Rating
Not sure what you are asking. Trying to put a complete SPI interface into a single component sounds like a nightmare.

I have done an SPI interface, and here's what I've done (right or wrong). It was designed specifically for interfacing to a flash memory, so it's a little specialized, as it has to handle block transfers and handshaking with the flash. The top level is a single component which handles the interface to the rest of the design, call it SPI.VHD. It is composed of several lower-level components, primarily a state machine which handles the interface between an input module, an output module and the upper level.

There is no single answer to your question and no 'right' way to do it (although academics will probably say I'm wrong). The particular design kind of dictates how you should partition it. There's also personal preference; that's where the 'art' comes into play.
 
Reactions: shaiko

    shaiko

    Points: 2
    Helpful Answer Positive Rating
There is no single answer to your question and no 'right' way to do it (although academics will probably say I'm wrong).
I never said that there's a "right" way to do it.

The particular design kind of dictates how you should partition it.
That's why I said - "when writing reusable logic blocks" (as opposed to logic customized to a very special system)

There's also personal preference; that's where the 'art' comes into play.
Exactly! And I'm asking about the personal preference of users in this forum.
 

So eventually you end up with a hierarchical structural design of a netlist? If you are designing down to the gate level for all your logic and aren't letting a synthesis tool do the work, then you must have a very forgiving job/boss that doesn't care about schedules or meeting deadlines to produce a product.

If I did that at any of the jobs I've had I would have been put on the short list of next in line for layoffs. The easiest way to get a quick estimate of size and logic levels is to just run the module through synthesis, not creating a gate level description. It makes me think that your exposure to FPGA/ASIC design is primarily in academia, outside academia the primary objective is to produce something before the competition or within a certain schedule to meet a customer requirement. You certainly don't want to waste time doing what a synthesis tool already does for you.
 
Reactions: shaiko

    shaiko

    Points: 2
    Helpful Answer Positive Rating
ads-ee,

I agree with ads-ee. I didn't expect to hear any "down to the gate" answers myself. However, as this is a "flavor" question I'd like to avoid judging other people's "art".

ads-ee,
If I take a look at 80% of your "general purpose" designs - what type of components will I see in the lowest hierarchies?
 

Hi ads-ee. I am very much part of the industry. I am of the belief that designing is a better challenge than just writing RTL code. As you get used to it, you tend to do things quicker. I definitely would prefer to do things this way.
But I do agree with you that customer requirements are of utmost importance. So sometimes you do tend to follow the alternative you mentioned in your last sentence..
 

If I take a look at 80% of your "general purpose" designs - what type of components will I see in the lowest hierarchies?
The lowest level stuff you'll see is common library components for stuff like synchronizers, edge detectors, hamming codes, etc. For other stuff I sort of use a standard of keeping a file to 200 lines or less of code (ignoring comments) give or take 100 lines ;-). The only file that doesn't adhere to that standard are top level files for a large chunk of a design, which is made up of a bunch of instantiated submodules. In those cases I may have 1000+ line files.

What I don't do is arbitrarily break up a file into submodules just because a file starts getting large if there aren't any good boundaries to separate out chunks of logic. I've seen way to many designs turned into maintainence nightmares due to dividing up the logic into it's constituent components, shift registers, counters, etc.

Seriously is this.

Code Verilog - [expand]
1
shift_reg  shift_reg_i (.shift_out(shift_data), .shift_in({shift_data[N-2:0], new_data}), .clk(clk));



better than this?

Code Verilog - [expand]
1
2
always @ (posedge clk)
  shift_data <= {shift_data[N-2:0], new_data};



what if shift_reg is defined as this:

Code Verilog - [expand]
1
2
always @ (posedge clk)
  shift_data <= {new_data, shift_data[N-1:1], };



Now I have to go look at shift_reg to make sure it's shifting the correct direction when it would have been less trouble and 1 extra line (formatted nicely) to see it in the inline code.
 
Reactions: shaiko

    shaiko

    Points: 2
    Helpful Answer Positive Rating

Code Verilog - [expand]
1
2
always @ (posedge clk)
  shift_data <= {shift_data[N-2:0], new_data};


This one gets my vote. Readable, and immediately apparent what is going on.

An exception to this would be if I needed extra control over the specific resources for the shift register, and especially when I need a whole bunch of them.

Say when you want the SRL juuuust over there, and then a buffer flip-flop right there. Then you probably want a module that neatly abstracts that away. Otherwise you get too many inline constraints making it a bit unwieldy.

Ah crap, this just made me realize I have to test how that works in Vivado. Different thread it is!
 
Reactions: shaiko

    shaiko

    Points: 2
    Helpful Answer Positive Rating
Say when you want the SRL juuuust over there, and then a buffer flip-flop right there. Then you probably want a module that neatly abstracts that away. Otherwise you get too many inline constraints making it a bit unwieldy.
And that is why I have a common library of synthesizable code that is parameterized and allows specifying all those special cases. So if I need something that specific I pull it from the library and bingo I know by looking at the code that it's from the library and it's not the simple version.
 

Yes
2. Will you divide it to components (shift register, clock generator , etc...) in several files ?
No. Something as simple as a shift register is just code in the file. For a SPI peripheral I don't know what sort of 'clock generator' there would be. However, for a SPI Controller, the 'clock generator' wouldn't really exist as such. Instead it is simply a counter where logic will use that counter value to do various things such as generating the SPI clock, enabling when input data is captured, allowing output data to change, etc.

For a more complicated example such as a Jpeg encoder, there would be separate files. The files however would break down along functional boundaries for the encoder such as 'entropy encoder', 'DCT', 'Coefficient Mapping', 'Huffman Lookup'. In other words, functional entities that map back to a specification. The source files would not be broken down to correspond to hardware thingies such as counters or clock generators.

3. Will you go even further down and describe the clock generator as a counter and comparator ?
Not quite sure what you're asking here but yes the counter is described with a comparator. The upper limit on that counter is defined by two generic inputs to the entity: Clock_Period (the period of the FPGA's internal clock that is clocking the logic) and Spi_Clock_Period (which is the desired Spi clock period). Both are generic inputs of type 'time'. The ratio determines the upper bound of the counter. From that upper bound other constants that are useful to the Spi Controller are then computed such as the count that corresponds to when the rising edge should happen, the falling edge should happen, etc. (by the way, this is for the Spi controller, a Spi device would not need these since it is simply a slave to whatever the external Spi controller does). A Spi peripheral generally only needs to detect changes in the input Spi clock and then do something. To detect a change in the Spi clock one would simply compare the previous state to the current state, there would be no need for a 'clock generator'.

Kevin Jennings
 
Reactions: shaiko

    shaiko

    Points: 2
    Helpful Answer Positive Rating

Indeed. I do the exact same thing. If it's simple then it's simple, and you had better KEEP it simple. What I do miss in the typical IDE is an easy way to quickly follow the definitions/declarations with a single key press. HDL tools are years behind compared to the software industry. Or at least the tools that I can afford from both camps.
 

I've seen way to many designs turned into maintainence nightmares due to dividing up the logic into it's constituent components, shift registers, counters, etc.

IMO, dividing up the logic into it's constituent components is the most "elegant" approach from an architectural point.
I also try to keep low the number of types of "atomics". I.E, I tend to use the same shift register component in all of my designs.

The good: fewer number of components in my library.
The bad: to make it versatile and suitable for every need, each component tends to be complex.

- - - Updated - - -

K-J,
For a SPI peripheral I don't know what sort of 'clock generator' there would be. However, for a SPI Controller, the 'clock generator' wouldn't really exist as such.
When I said SPI peripheral - I meant ALL the logic that's required for a fully featured master & slave (including the controller).
 

I dont really care how many files it is.
As a user, my main concerns are:

1. Does it work as per the spec?
2. Does it have good documentation? (this included written docs and comments)
3. Will I be able to fix bugs in it having never worked on it before (no2 is very important here)

If it's all made in the gate level, then no. 3 may not be very easy.
 

The bad: to make it versatile and suitable for every need, each component tends to be complex.

Yup. And sometimes this bad can become very bad. And then you pay the price for overgeneralization. And then you get sick of paying the price for overgeneralization. And then you keep things simple again.

Trying to do everything for every possible scenario ever will result in unwieldy components really fast.

I'd rather have 3 super simple shift register modules of different flavors than 1 shift register module that can do all 3 (using parameters), but becomes rather complex due to having to handle all 3 cases. One extra parameterized module acting purely as a wrapper for the other 3 might be acceptable. That is, until we get proper template support a la C++. Because no-one ever got a headache because of templates in C++.
 
Reactions: shaiko

    shaiko

    Points: 2
    Helpful Answer Positive Rating
I don't see anything "elegant" about having a bunch of shift registers, counters and glue logic scattered around, when you can put all that stuff into a single, well-verified module that provides a simple interface. For instance, an "SPI input" module that would have an 8-bit data output, a data-ready output signal, a read signal input, and an SPI serial input would be a LOT more elegant than a separately instantiated shift register, data latch, ready logic, etc.
 

I don't see anything "elegant" about having a bunch of shift registers, counters and glue logic scattered around, when you can put all that stuff into a single, well-verified module that provides a simple interface.

Agreed. The important part being where you put your interface boundaries, as in how you chop up your total design into smaller modules. Do you want to put every single element into it's own module? Probably not.

In defense of those "bits scattered around", I'd assume them to be instantiated from within the well-verified module. But you'd get an extra level of hierarchy since then your shift registers, counters etc would then be sub-modules of said well-verified module.
 

Yup. And sometimes this bad can become very bad. And then you pay the price for overgeneralization. And then you get sick of paying the price for overgeneralization. And then you keep things simple again.
And then when you end up with 256 types of SPI modules you start generalizing again ?
 

Well the fact that most people write code like this:


Code Verilog - [expand]
1
2
3
a a_i (xyz, abc, nmo, pqr, c, r);
b b_i (c, r, abc, xyz, hij);
add add_i (hij, xyz, add_out);



makes it extremely painful to deal with code that is hidden behind all those submodules without even named port mappings and names that s**k on top of everything else. Oh and two lines of comments at the top of the file...name of file (duh I know the file name), and their name (like I would ever put my name in a file full of cr*p ;-))

As you may have figured out. I've gotten stuck with fixing a lot of bad designs inherited from people that were: laid off, quit (before the design is used), shouldn't have been given the task in the first place, etc. (i.e. basically incompetent nincompoops that shouldn't be allowed to design anything!)

Besides that having code written in a structural manner makes me have to really examine the code carefully as there isn't any behavior specified, so IMO it's less of a higher level implementation, which is the whole point of using an HDL, I might as well go back to drawing schematics at that point as it's easier and faster for me that way than drawing an HDL schematic description. The quest for improving productivity is the drive behind using higher levels of abstraction, hence the development tools to convert C to RTL, so you can move up to a higher level of abstraction.
 
Reactions: shaiko

    shaiko

    Points: 2
    Helpful Answer Positive Rating
And then when you end up with 256 types of SPI modules you start generalizing again ?

No. Because 256 = 2^8. I only do prime numbers.

When you end up with 256 types of SPI modules it's probably you only have 1 type of SPI module with an 8-bit parameter.

What I meant with the 3 flavors is that you really have 3 flavors of something that are somewhat similar (certainly high level functionality similar), but have distinct differences in processing. So not just different bit-width or some such, but really different processing, different branches etc.
 
Reactions: shaiko

    shaiko

    Points: 2
    Helpful Answer Positive Rating
Status
Not open for further replies.

Similar threads

Cookies are required to use this site. You must accept them to continue using the site. Learn more…