UPGRADE YOUR BROWSER

We have detected your current browser version is not the latest one. Xilinx.com uses the latest web technologies to bring you the best online experience possible. Please upgrade to a Xilinx.com supported browser:Chrome, Firefox, Internet Explorer 11, Safari. Thank you!

cancel
Showing results for 
Search instead for 
Did you mean: 
Adventurer
Adventurer
10,861 Views
Registered: ‎11-12-2009

FSM's - why and their synthesis?

Jump to solution

Hi, A theory question really but would love to know what someone more experienced thinks about these things.

 

Firstly, why use a FSM when a case statement would do just fine?

Is this purely a readability issue for when your case/state list gets too long and complicated for ease? Or is there a hardware reason for using FMS's, does it create more efficient hardware if the synthesizer picks up you're using a FSM? I want to ask this ignoring the encoding strategies issue, I know the synthesizer can potentially help me and reduce my resource usage if I let it chose an encoding in a FSM but apart from that (as I can self implement such in a case statement if necessary) what difference in hardware does it make?

 

A second question leading on from that, if there is a fundamental efficiency reason to use a FSM instead of a case statement say, then how important is it that my synthesizer works out that that's what I'm trying to infer (again ignoring the encoding  point)? I'm asking as in some cases, following the template in order to have the synthesizer recognise a FSM, produces very complex unreadable code which could be made much simpler but if done so the synthesizer fails to infer a FSM specifically, (even though functionally it works great) There isn't any native "FSM hardware" as such like DSP's or BRAM etc, so what's the difference and why does the synthesizer tell me it's inferring a FSM?

 

Much thanks

0 Kudos
1 Solution

Accepted Solutions
Advisor eilert
Advisor
13,211 Views
Registered: ‎08-14-2007

Re: FSM's - why and their synthesis?

Jump to solution

Hi,

a case statement in the end is some kind of ROM.

It can do more than that, but let's keep it simple for now.

 

You can use feed the case selector e.g. from a counter to create repeating output patterns, almost like a FSM would do.

But The FSM is capable of "nonlinear" behavior, e.g. change the output patterns according to different input patterns over time.

A case that can handle that would grow extremely big. You would have to describe all the possible pattern that could be derived from all possible input patterns over time. Almost impossible.

 

There are several approved coding styles for FSMs, which have a good readability.

If your FSM code goes weird, you probably haven't completely analyzed the problem and done the necessary optimizations for your systems architecture.

e.g., - you can break up a large complex FSM inot several small ones.

        - Don't include the datapath (arithmetic operations ) in yourt FSM.

and so on...

 

Look for the Picoblaze, it's a programmable (F)SM: KCPSM

 

Have a nice synthesis

  Eilert

 

0 Kudos
51 Replies
Advisor eilert
Advisor
13,212 Views
Registered: ‎08-14-2007

Re: FSM's - why and their synthesis?

Jump to solution

Hi,

a case statement in the end is some kind of ROM.

It can do more than that, but let's keep it simple for now.

 

You can use feed the case selector e.g. from a counter to create repeating output patterns, almost like a FSM would do.

But The FSM is capable of "nonlinear" behavior, e.g. change the output patterns according to different input patterns over time.

A case that can handle that would grow extremely big. You would have to describe all the possible pattern that could be derived from all possible input patterns over time. Almost impossible.

 

There are several approved coding styles for FSMs, which have a good readability.

If your FSM code goes weird, you probably haven't completely analyzed the problem and done the necessary optimizations for your systems architecture.

e.g., - you can break up a large complex FSM inot several small ones.

        - Don't include the datapath (arithmetic operations ) in yourt FSM.

and so on...

 

Look for the Picoblaze, it's a programmable (F)SM: KCPSM

 

Have a nice synthesis

  Eilert

 

0 Kudos
Instructor
Instructor
10,836 Views
Registered: ‎08-14-2007

Re: FSM's - why and their synthesis?

Jump to solution

First of all, a finite-state machine is a pretty broad term that generally includes any

sequential logic with a finite number of states such as a counter.  However to the

synthesis tools there is another meaning, which is a finite state machine which

the tools don't recognize as a simple state machine like a counter, and that will

be therefore encoded using some different form of optimization.  Usually those

state machines that are reported as such are encoded "one-hot".  This means

that you have a flip-flop for every reachable state and only one of these has the value

1 at any given time.  It also means that your "default" statement as well as any

code applied to non-reachable states is not synthesized, so if the state machine

goes haywire it may not recover.  You can force synthesis to use other encodings

as well as "safe" encodings that take care of stuck or illegal conditions.

 

Regards,

Gabor

-- Gabor
0 Kudos
Participant alexium
Participant
9,772 Views
Registered: ‎04-22-2010

Re: FSM's - why and their synthesis?

Jump to solution

 


@eilert wrote:

 

        - Don't include the datapath (arithmetic operations ) in yourt FSM.

 


 

Why not?

Are you saying it's better to have the control automaton (FSM itself)  and operational automaton (the thing that performs operations, ALU, for example) separate rather than describing them in the same case statement of the same process?

 

0 Kudos
Teacher eteam00
Teacher
9,769 Views
Registered: ‎07-21-2009

Re: FSM's - why and their synthesis?

Jump to solution

alexium wrote:

eilert wrote: 

        - Don't include the datapath (arithmetic operations ) in yourt FSM.


Why not?

Are you saying it's better to have the control automaton (FSM itself)  and operational automaton (the thing that performs operations, ALU, for example) separate rather than describing them in the same case statement of the same process?


Alexium:

 

1.  There are a number of different coding styles for state machiines which are popular.  We are all entitled to our own style choices, so no-one should take eilert's advice as more than a suggestion (rather than a warning of impending doom).

 

2.  The suggestion is in response to a broad inquiry, with very little context or explanation to elaborate when or where or [to what extent] the advice should be applied.

 

3.  This thread is dormant for almost a year (until you awakened it).  For all we know, this fellow   doesn't even hang out around here anymore -- who knows if he's been promoted from design and development to product marketing... -- you may be waiting a while for a response.

 

-- Bob Elkind  :)

SIGNATURE:
README for newbies is here: http://forums.xilinx.com/t5/New-Users-Forum/README-first-Help-for-new-users/td-p/219369

Summary:
1. Read the manual or user guide. Have you read the manual? Can you find the manual?
2. Search the forums (and search the web) for similar topics.
3. Do not post the same question on multiple forums.
4. Do not post a new topic or question on someone else's thread, start a new thread!
5. Students: Copying code is not the same as learning to design.
6 "It does not work" is not a question which can be answered. Provide useful details (with webpage, datasheet links, please).
7. You are not charged extra fees for comments in your code.
8. I am not paid for forum posts. If I write a good post, then I have been good for nothing.
Participant alexium
Participant
9,759 Views
Registered: ‎04-22-2010

Re: FSM's - why and their synthesis?

Jump to solution

 


@eteam00 wrote:

 

3.  This thread is dormant for almost a year (until you awakened it).  For all we know, this fellow   doesn't even hang out around here anymore -- who knows if he's been promoted from design and development to product marketing... -- you may be waiting a while for a response.

 


 

I thought I'd rather ask in this thread instead of creating new, is it a bad choice? And I have no knowledge of  's destiny, but I'm quite sure he answered my threads about a week ago...

What I would like know is whether particular FSM coding style has performance or reliability advantage over other styles (synthesis-wise), and, specifically, may I include the datapath in the FSM case statement or should I avoid it.

Thanks for the answer!

0 Kudos
Teacher eteam00
Teacher
9,756 Views
Registered: ‎07-21-2009

Re: FSM's - why and their synthesis?

Jump to solution

I thought I'd rather ask in this thread instead of creating new, is it a bad choice? And I have no knowledge of  's destiny, but I'm quite sure he answered my threads about a week ago...

It might have been his ghost...  or are you saying that his account has been hacked?

What I would like know is whether particular FSM coding style has performance or reliability advantage over other styles (synthesis-wise), and, specifically, may I include the datapath in the FSM case statement or should I avoid it.

Your skills and talents as a thoughful and meticulous designer will affect performance and reliability much more than coding style.  As you gain experience, your style will evolve to benefit from your greater understanding, comfort, and familiarity with design work.  If you work at it long enough, it's a possibility that your coding style may evolve over the years to resemble eilert's style in many ways.  Or maybe not.

 

The important thing is to code in a style which is logical to you.  I've known a few design engineers over the years.  Some are great abstract thinkers but fall short in details.  Some are detail nerds, but are weak in overall 'vision'.  Some are successful through sheer grit and determination and persistence.  If you have a preference for FSM coding style which is comfortable for you, don't be too quick to start from scratch with a completely unfamiliar style in deference to someone else -- even if the someone else is as respected as eilert (if he's still around).

 

- Bob Elkind

SIGNATURE:
README for newbies is here: http://forums.xilinx.com/t5/New-Users-Forum/README-first-Help-for-new-users/td-p/219369

Summary:
1. Read the manual or user guide. Have you read the manual? Can you find the manual?
2. Search the forums (and search the web) for similar topics.
3. Do not post the same question on multiple forums.
4. Do not post a new topic or question on someone else's thread, start a new thread!
5. Students: Copying code is not the same as learning to design.
6 "It does not work" is not a question which can be answered. Provide useful details (with webpage, datasheet links, please).
7. You are not charged extra fees for comments in your code.
8. I am not paid for forum posts. If I write a good post, then I have been good for nothing.
Participant alexium
Participant
9,749 Views
Registered: ‎04-22-2010

Re: FSM's - why and their synthesis?

Jump to solution

 


@eteam00 wrote:
The important thing is to code in a style which is logical to you.

 


  I do use a style that is logical to me, but there is a problem: my code misbehaves in hardware, and my tutor told me  synthesis tool didn't synthesise my FSM properly  because I didn't stick with 3-process FSM template and because I didn't separate the datapath from the FSM. I seriously doubt my tutor is right, but I'd rather ask someone experienced. All in all, I have no other ideas as to why doesn't my code work...

 

0 Kudos
Teacher eteam00
Teacher
9,740 Views
Registered: ‎07-21-2009

Re: FSM's - why and their synthesis?

Jump to solution

I do use a style that is logical to me, but there is a problem: my code misbehaves in hardware, and my tutor told me  synthesis tool didn't synthesise my FSM properly  because I didn't stick with 3-process FSM template and because I didn't separate the datapath from the FSM.

I devoted most of my few remaining brain cells to a long-winded exposition on why divergence in coding style is perfectly fine.  Having said that, a 3-process state machine sounds somewhat bizarre and unwieldy.  So much for design style tolerance!

 

On the other hand, if the '3rd process' is nothing more than an ALU datapath, and all the control logic is embodied in a single process, I would not necessarily consider this a '3-process state machine'.  We may be discussing semantic details rather than substantive design differences.

I seriously doubt my tutor is right, but I'd rather ask someone experienced. All in all, I have no other ideas as to why doesn't my code work...

Well, I'm definitely experienced (I'm practically dead).  Here's an idea...

 

Set your tutor's style aside for the moment, and write your code the way you see fit.  See what happens.  As long as you stick to fewer than 4 processes per state machine.

 

You might consider posting your code for critique and review.

And then there's simulation as a debugging tool.  FSMs are quite friendly to debugging-by-way-of-simulation.

 

-- Bob Elkind

SIGNATURE:
README for newbies is here: http://forums.xilinx.com/t5/New-Users-Forum/README-first-Help-for-new-users/td-p/219369

Summary:
1. Read the manual or user guide. Have you read the manual? Can you find the manual?
2. Search the forums (and search the web) for similar topics.
3. Do not post the same question on multiple forums.
4. Do not post a new topic or question on someone else's thread, start a new thread!
5. Students: Copying code is not the same as learning to design.
6 "It does not work" is not a question which can be answered. Provide useful details (with webpage, datasheet links, please).
7. You are not charged extra fees for comments in your code.
8. I am not paid for forum posts. If I write a good post, then I have been good for nothing.
Participant alexium
Participant
9,732 Views
Registered: ‎04-22-2010

Re: FSM's - why and their synthesis?

Jump to solution

 


@eteam00 wrote:
On the other hand, if the '3rd process' is nothing more than an ALU datapath

 

I believe it is only true when the datapath is purely combinatorial. Otherwise it should be within a clocked process, which is a deviation from the template.

 

 


@eteam00 wrote:

And then there's simulation as a debugging tool.  FSMs are quite friendly to debugging-by-way-of-simulation.

 


By saying it misbehaves in hardware I mean it simulates perfectly fine, and there are no synthesis warnings that might point to a problem.

I would very much like to have someone look through my code, but I'm not sure if it is short enough. Here is the code for one of the two modules in question. I'll appreciate it very much if you  take a brief look at the code and try to find any poor techniques I'm using.

Anyways, thanks a lot for your expanded answers!

 


@eteam00 wrote:

a 3-process state machine sounds somewhat bizarre and unwieldy


Not only it sounds that way, but such a style really complicates both the visual analysis and the further modification of an FSM (due to the necessity of jumping from one process to another, and back, and then forth again, and so on over and over again... )

 

 


@eteam00 wrote:

 

In RXLoop, the 'waiting' state is never reached.

 


That is the intention. I might need it in future. Should have also removed in from the code uploaded...

 

0 Kudos
Teacher eteam00
Teacher
9,179 Views
Registered: ‎07-21-2009

incomplete code review

Jump to solution

If the design 'behaves' well in simulation, but not in real operation, there are two likely causes:

1.  Incomplete simulation testbench (in other words, you are missing some design coverage in your tests)

2.  clock domain crossing problem (async input not properly synchronised to clocked logic)

 

In your linked code page, would you please direct our attention to the interesting 'problem' code which you would like us to review?  You declare some modules -- modules which no doubt include state machines or clocked logic -- but the modules themselves are missing from the displayed code.  In other words, your code listing is incomplete.

 

With the incomplete code on the webpage, my first and most excited reaction is to question the use of 2 clocks -- 100MHz and 50MHz.  There is nothing in your logic which suggests that both clocks are necessary, and the presence of two clock domains can cause no end of problems -- especially clock domain crossing problems which would not (necessarily) show up in simulation.

 

-- Bob Elkind

SIGNATURE:
README for newbies is here: http://forums.xilinx.com/t5/New-Users-Forum/README-first-Help-for-new-users/td-p/219369

Summary:
1. Read the manual or user guide. Have you read the manual? Can you find the manual?
2. Search the forums (and search the web) for similar topics.
3. Do not post the same question on multiple forums.
4. Do not post a new topic or question on someone else's thread, start a new thread!
5. Students: Copying code is not the same as learning to design.
6 "It does not work" is not a question which can be answered. Provide useful details (with webpage, datasheet links, please).
7. You are not charged extra fees for comments in your code.
8. I am not paid for forum posts. If I write a good post, then I have been good for nothing.
Participant alexium
Participant
9,172 Views
Registered: ‎04-22-2010

Re: incomplete code review

Jump to solution
There are two "black box" components in the code: the Buffer is a dual-port RAM from Xilinx CoreGen, and the UART is downloaded from OpenCores and modified (by me) to reduce resource usage. After the modification, the UART has been tested quite thoroughly and have always functioned well.
As for two clocks - you're right, I should have removed the unused 50_MHZ clock from the port list, because it really is never used (nowhere in the whole design).

And I would very much like to point out the specific region where a problem seems to be localized, but I can't. The bug is very evasive, unstable and ... uhm... ghosty. i can't localize it.
The module I've shown to you is included into another, top-level module, which is about the same number of lines and has one complex FSM. Any minor modification to either of these 2 modules might (or might not) trigger the bug. For example, assigning a constant number to a "debug" signal in every (or even some) states of FSM triggers the bug. Also, the bug can be enabled or disabled by changing the synthesis options or the mapping options!
0 Kudos
Instructor
Instructor
9,166 Views
Registered: ‎08-14-2007

Re: incomplete code review

Jump to solution

This sounds like a classic case of asynchronous (clock domain crossing) logic killing

a state machine.

 

Here are some things to think about:

 

When you have an asynchronous signal (whether external or from another part of your design

but synchronous to a different clock), it should only cross into a new clock domain (go through

a flip-flop clocked by the new clock) in 1 place.  Many people learn this lesson the hard way,

and often start with the misconception that what they are seeing is "metastability," a much

more rare problem.  What happens when two or more flip-flops in clock domain B both

receive a signal from clock domain A, then when the signal changes state near the active

edge of clock B, one flip-flop may see the old value of the signal while the other already

sees the new value.  So for one clock cycle these two flops are out of synch with eachother.

 

Now suppose these flops are actually state flops in a one-hot encoded state machine.

In that machine you have state FOO which transitions to state BAR only when signal

A is high.  Now imagine that A changes very near the clock edge (of clock B), and

due to path delay differences, state flop FOO samples signal A high, while on the

same cycle flop BAR samples A low.  Because of the way the one-hot machine is

implemented, this means that the state machine will leave state FOO (because

that flop saw A go high), but it does not enter state BAR (because that flop did not

see A go high).  So now the state machine becomes "zero-hot" meaning none

of its state flops are active.  Furthermore because state BAR not only depends on

the state of signal A, but also on the state of flop FOO, it will not become active on

the following cycle when it also senses that A has gone high.  (are we dizzy yet?)

 

So the only fix to the above scenario is to register signal A in a separate flop

(outside the state machine) and use the output of that flop, now synchronous

to clock B, inside the state machine.

 

The main reason why the symptoms of the above problem can come and go, is

that from run to run the placement varies and with it can also vary the relative

path delay of signal A to flop FOO vs. signal A to flop BAR.  For any placement

where flop BAR sees the signal sooner, you don't get the "zero-hot" state but

instead get a "multihot" state (both FOO and BAR) which may not exhibit

any outward symptoms (for example state FOO only waits for signal A so

having no other side-effects it doesn't impeded the normal operation of

state BAR).

 

Think about it...

 

-- Gabor

-- Gabor
Participant alexium
Participant
9,160 Views
Registered: ‎04-22-2010

Re: incomplete code review

Jump to solution

 


gszakacs wrote: 

are we dizzy yet?

 


No, I believe I am folowing the thought.

 

Now I really understand why should async signals be resynchronized to an active clock.

But the thing is I have one sibgle clock for the whole design and I have absolutely no async inputs to an FSM. And yet, the design seems to be very sensitive to placement (or routing, or both). I have know idea as to where the problem might come from.

P.S. I have always wondered, how come different traces length of different interconnections between LUTs/Flops inside FPGA doesn't usually cause the problem of signal change edge mismatching the clock edge. I believe it is due to flip-flops being two-stage, but still it doesn't cease to amaze me...

 

0 Kudos
Xilinx Employee
Xilinx Employee
9,157 Views
Registered: ‎01-03-2008

Re: incomplete code review

Jump to solution

> But the thing is I have one sibgle clock for the whole design and I have absolutely no async inputs to an FSM.

 

Take a look at your synthesis, MAP and PAR reprots.  You likely have unintended latches in your design.

------Have you tried typing your question into Google? If not you should before posting.
Too many results? Try adding site:www.xilinx.com
0 Kudos
Participant alexium
Participant
9,154 Views
Registered: ‎04-22-2010

Re: incomplete code review

Jump to solution

 


@mcgett wrote:

> But the thing is I have one sibgle clock for the whole design and I have absolutely no async inputs to an FSM.

 

Take a look at your synthesis, MAP and PAR reprots.  You likely have unintended latches in your design.


 

Have checked it right away. I have zero number of latches...

 

0 Kudos
Xilinx Employee
Xilinx Employee
9,148 Views
Registered: ‎01-03-2008

Re: incomplete code review

Jump to solution

No latches, no asynchronous inputs, single clock and all timing constraints are met?  Then there shouldn't be a timing issue and the design should be working as intended.

 

How is your design reset?  Asynchronous, synchronous, or only by GSR during configuration?

What does your actually do?  I saw a comment about a UART and a BlockRAM, but not else.

Does your design pass your simulation test bench?  How robust is your simulation test bench?

------Have you tried typing your question into Google? If not you should before posting.
Too many results? Try adding site:www.xilinx.com
Teacher eteam00
Teacher
9,134 Views
Registered: ‎07-21-2009

Re: incomplete code review

Jump to solution

Now I really understand why should async signals be resynchronized to an active clock.

There is no synchronisation logic in your posted code.

But the thing is I have one sibgle clock for the whole design and I have absolutely no async inputs to an FSM.

You use two clocks in your posted code: clk_50M and clk_100M.

 

You do have async inputs to a FSM

  • the RX_PHY uart input
  • possibly rst
  • all signals passed between clk_50M and clk_100M clock domains

You are getting the same advice and comments from mcgett, from gabor, and from me.  Who else are you going to ask?

If you posted more of your code, we might be able to point out specific problems.

 

- Bob Elkind

SIGNATURE:
README for newbies is here: http://forums.xilinx.com/t5/New-Users-Forum/README-first-Help-for-new-users/td-p/219369

Summary:
1. Read the manual or user guide. Have you read the manual? Can you find the manual?
2. Search the forums (and search the web) for similar topics.
3. Do not post the same question on multiple forums.
4. Do not post a new topic or question on someone else's thread, start a new thread!
5. Students: Copying code is not the same as learning to design.
6 "It does not work" is not a question which can be answered. Provide useful details (with webpage, datasheet links, please).
7. You are not charged extra fees for comments in your code.
8. I am not paid for forum posts. If I write a good post, then I have been good for nothing.
Participant alexium
Participant
9,116 Views
Registered: ‎04-22-2010

Re: incomplete code review

Jump to solution

I do not have two clocks! I have a signal named CLK_50M which is never used, nowhere in the whole design. There is only one clock -CLK_100M. Like I said, I should have removed it before posting the code, sorry for confusing you.

As for RX_PHY - you're right, but the UART has really been tested extensively before incorporating into this module.

As for rst - yes, it is async, but from the way design behaves I know the problem does not come from reset circuit, I'm 100 percent sure of that, so I left it asynchronous.

Please don't get angry with me, I am listening to your advices! I am not stupid (well, not THAT stupid), and I have no intention or desire of wasting your time in vain by asking the same question again and again. But my design is simpler than it might have seemed to you, your advices are mostly related to complex design techniques I don't use at all.

 

And  the initial question was whether it is appropriate (for synthesis) to write FSMs the way I do, or not.

0 Kudos
Participant alexium
Participant
9,107 Views
Registered: ‎04-22-2010

Re: incomplete code review

Jump to solution

 


@mcgett wrote:

No latches, no asynchronous inputs, single clock and all timing constraints are met?  Then there shouldn't be a timing issue and the design should be working as intended.


 

No latches, no async inputs (except for UART receiving line, but that's being dealt with inside the UART module). I have only one timing constraint - PERIOD - and it is met by wide margins.

The reset is asynchronous, but, like I said, I know for sure  reset is not the cause of my problem.

The design does pass the TB perfectly, and the TB covers all (few) possible module's actions.

0 Kudos
Teacher eteam00
Teacher
9,257 Views
Registered: ‎07-21-2009

Re: incomplete code review

Jump to solution

I do not have two clocks! I have a signal named CLK_50M which is never used, nowhere in the whole design.

Perhaps I'm needlessly confused.  If the following does not reflect portions of your code (per your link), please correct me (or you).

 

RXBuf: Buf
    port map(
    clka => clk_50M,
    wea => "1",
    addra =>RXAddrA,      -- generated by clk_100M
    dina => RXByte_int,   -- generated by clk_100M
    clkb => clk_50M,
    addrb => RXAddrB,     -- generated by clk_100M
    doutb => RXByte       
);

TXByteAddrCorrected <= TXByteAddrInPacket-1;        --!    

TXAddrA <=  conv_std_logic_vector(TXByteAddrCorrected, 11);
TXAddrB <= conv_std_logic_vector(TXByteNumber, 11);
WE<= "0" when TXWriteByte = '0' else "1";
TXBuffer: Buf
    port map(
    clka    => clk_50M,
    wea     => WE,        -- generated by clk_100M
    addra   => TXAddrA,   -- check generation of this address!
    dina    => TXByte,    -- check source clock domain
    clkb    => clk_50M,
    addrb   => TXAddrB,   -- generated by clk_100M
    doutb   => TXByte_int -- used by clk_100M
);

As for RX_PHY - you're right, but the UART has really been tested extensively before incorporating into this module.

As for rst - yes, it is async, but from the way design behaves I know the problem does not come from reset circuit, I'm 100 percent sure of that, so I left it asynchronous.

I feel like we are staging an 'intervention' in your life.  You seem to be in denial.

Is your design working well and reliably, or is it not?

 

Are you telling us that

  • portion of your design with unsynchronised inputs to a FSM is OK... because it is extensively tested
  • your design is working well in simulation
  • but your design is not working well in real hardware

Please don't get angry with me, I am listening to your advices!

  • We're not angry.
  • We're trying to help you.

'Listening to our advice' would include

  • clean up your clocks (1 is good, 2 requires sync logic, suggest you stick to 1 clock)
  • clean up your async inputs (including reset)
  • re-build the design, and re-test

And  the initial question was whether it is appropriate (for synthesis) to write FSMs the way I do, or not.

Without seeing the FSM code in question, the discussion won't be very long or informative.

Show us how horrible (or strange or wonderful) the code is, and you'll have no shortage of opinions.

I would very much like to have someone look through my code, but I'm not sure if it is short enough. Here is the code for one of the two modules in question. I'll appreciate it very much if you  take a brief look at the code and try to find any poor techniques I'm using.

I think we've answered you to the extent your posted code sections permit.  We can re-phrase the responses, but the underlying comments and ideas will remain.

 

-- Bob Elkind

SIGNATURE:
README for newbies is here: http://forums.xilinx.com/t5/New-Users-Forum/README-first-Help-for-new-users/td-p/219369

Summary:
1. Read the manual or user guide. Have you read the manual? Can you find the manual?
2. Search the forums (and search the web) for similar topics.
3. Do not post the same question on multiple forums.
4. Do not post a new topic or question on someone else's thread, start a new thread!
5. Students: Copying code is not the same as learning to design.
6 "It does not work" is not a question which can be answered. Provide useful details (with webpage, datasheet links, please).
7. You are not charged extra fees for comments in your code.
8. I am not paid for forum posts. If I write a good post, then I have been good for nothing.
Participant alexium
Participant
9,245 Views
Registered: ‎04-22-2010

Re: incomplete code review

Jump to solution

 


@eteam00 wrote:

Please don't get angry with me, I am listening to your advices!

 

  • We're not angry. 

 


I hope not, but I couldn't blame you if you are, considering my fuzzy answers. I'll try to clarify the situation.


@eteam00 wrote:

I do not have two clocks! I have a signal named CLK_50M which is never used, nowhere in the whole design.

Perhaps I'm needlessly confused.  If the following does not reflect portions of your code (per your link), please correct me (or you).

 


Ouch! That's my mistake, I have already replaced CLK_50 with CLK_100 there. But it doesn't really matter (right now), because both CLK_50 and CLK_100 ports are fed with the very same 50 MHz clock signal (really badplay on words, having CLK_100M signal that's actually 50MHz...). Sorry for that mistake.

 

I'd like to clarify the situation with async inputs  to FSM. As far as I understand, the only async input is UART RX line, which is only used unside the UART component. I had a test project, which only contained the UART module in question and some simple logic to test the UART. Using that test project, I have tested the UART before incorporating it to the actual design we're talking about now. Do you think it's likely that UART worked well in test design, but won't work in the current design?

 


@eteam00 wrote:

Are you telling us that

  • portion of your design with unsynchronised inputs to a FSM is OK... because it is extensively tested
  • your design is working well in simulation
  • but your design is not working well in real hardware

 


Yes, that's pretty much what I'm saying, except for the first statement I've tried to clarify in the above written.


eteam00 wrote:

'Listening to our advice' would include

  • clean up your clocks (1 is good, 2 requires sync logic, suggest you stick to 1 clock)
  • clean up your async inputs (including reset)
  • re-build the design, and re-test

Will do (except for the first point which doesnt apply to my design).

 


@eteam00 wrote:

Without seeing the FSM code in question, the discussion won't be very long or informative.

 


I thought  you did see it: RXLoop and TXLoop processes here each represent an FSM (quite an ugly one, but still).

 

 


@eteam00 wrote:
I think we've answered you to the extent your posted code sections permit.   

 


I believe you have.

Thanks again for the help!

 

 

 

 

0 Kudos
Teacher eteam00
Teacher
9,236 Views
Registered: ‎07-21-2009

Re: incomplete code review

Jump to solution

I don't see why you think RXLoop and TXLoop state machines are ugly.  What do you not like about them?

 

In RXLoop, the 'waiting' state is never reached.

 

I'm not a VHDL wizard.  My comments would be more credible if your code is Verilog.

 

In Verilog, I would avoid using identical labels for states in two different state machines within a single module.  For instance, I would not use waiting for both TXState and RXState (example: use TXwaiting and RXwaiting, respectively) .

I'd like to clarify the situation with async inputs  to FSM. As far as I understand, the only async input is UART RX line, which is only used unside the UART component. I had a test project, which only contained the UART module in question and some simple logic to test the UART. Using that test project, I have tested the UART before incorporating it to the actual design we're talking about now. Do you think it's likely that UART worked well in test design, but won't work in the current design?

Yes, it is a problem.  Spend 2 FFs to sync RX input, and don't ever do that again.

This is digital logic design 101 type of problem.

 

I honestly don't know what the problems with your design might be, but I am bothered that you have repeatedly stated that 'this logic is tested, so I know it can't be wrong, but it still does not work'.  This calls into question your test approach and skills.

 

If Xilinx told you that their software is 100% tested, and yet it still crashes under Windows XP, would you be skeptical?  Right...  I thought so.

 

-- Bob Elkind

SIGNATURE:
README for newbies is here: http://forums.xilinx.com/t5/New-Users-Forum/README-first-Help-for-new-users/td-p/219369

Summary:
1. Read the manual or user guide. Have you read the manual? Can you find the manual?
2. Search the forums (and search the web) for similar topics.
3. Do not post the same question on multiple forums.
4. Do not post a new topic or question on someone else's thread, start a new thread!
5. Students: Copying code is not the same as learning to design.
6 "It does not work" is not a question which can be answered. Provide useful details (with webpage, datasheet links, please).
7. You are not charged extra fees for comments in your code.
8. I am not paid for forum posts. If I write a good post, then I have been good for nothing.
Participant alexium
Participant
9,231 Views
Registered: ‎04-22-2010

Re: incomplete code review

Jump to solution

@eteam00 wrote:

I don't see why you think RXLoop and TXLoop state machines are ugly.  What do you not like about them?

 


I don't like the fact there are too few explicit states and the presence of the implicit states (a counter). Nd the again, the FSM doesn't correspond to any wide-known FSM template, so my tutor suggests it's not being synthesised correctly.

 

 


@eteam00 wrote:

 

I'm not a VHDL wizard.  My comments would be more credible if your code is Verilog.

 

 


Unfortunately, VHDL was the first HDL I've learnt... I understand Verilog code and can modify it, but I'm not comfortable writing in Verilog from scratch.

 


@eteam00 wrote:

I don't see why you think RXLoop and TXLoop state machines are ugly.  What do you not like about them?

 

In Verilog, I would avoid using identical labels for states in two different state machines within a single module.  For instance, I would not use waiting for both TXState and RXState (example: use TXwaiting and RXwaiting, respectively)


Good suggestion, thanks!

 

 


@eteam00 wrote:

 

I honestly don't know what the problems with your design might be, but I am bothered that you have repeatedly stated that 'this logic is tested, so I know it can't be wrong, but it still does not work'.  This calls into question your test approach and skills.

 

If Xilinx told you that their software is 100% tested, and yet it still crashes under Windows XP, would you be skeptical?  Right...  I thought so.

 


Makes sense. I'm currently working on improving the TB. But still, the TB only tests RTL code, that's all. The fact that error is elusive, and might be triggered not only by XST settings but also by Map settings, makes me think it's not a kind of problem that will definitely be caught during simulation.

 

 


@eteam00 wrote:

 

In RXLoop, the 'waiting' state is never reached.

 


That is the intention. I might need it in future. Should have also removed in from the code uploaded...

 

0 Kudos
Participant alexium
Participant
9,218 Views
Registered: ‎04-22-2010

Re: incomplete code review

Jump to solution

 


eteam00 wrote:

 Do you think it's likely that UART worked well in test design, but won't work in the current design?

Yes, it is a problem.  Spend 2 FFs to sync RX input, and don't ever do that again.

This is digital logic design 101 type of problem.

 


I can hardly believe my eyes, but that did it! At the very least it has greatly expanded the project's domain of stability!

 

Thank you very much!
Teacher eteam00
Teacher
9,206 Views
Registered: ‎07-21-2009

Re: incomplete code review

Jump to solution

From post #11 in this thread, written without having seen any of the posted code:

If the design 'behaves' well in simulation, but not in real operation, there are two likely causes:

1.  Incomplete simulation testbench (in other words, you are missing some design coverage in your tests)

2.  clock domain crossing problem (async input not properly synchronised to clocked logic)

Ed McGettigan and Gabor were quick to follow with much the same advice.

I guess we're not as dumb as we look.

 

Glad to hear your design is working better.  Even more glad if you've learned a lesson or two out of this.  If you need some explanation as to why asynchronous inputs to a state machine are so horrible, you should re-read Gabor's post (#13 in this thread).

 

-- Bob Elkind

SIGNATURE:
README for newbies is here: http://forums.xilinx.com/t5/New-Users-Forum/README-first-Help-for-new-users/td-p/219369

Summary:
1. Read the manual or user guide. Have you read the manual? Can you find the manual?
2. Search the forums (and search the web) for similar topics.
3. Do not post the same question on multiple forums.
4. Do not post a new topic or question on someone else's thread, start a new thread!
5. Students: Copying code is not the same as learning to design.
6 "It does not work" is not a question which can be answered. Provide useful details (with webpage, datasheet links, please).
7. You are not charged extra fees for comments in your code.
8. I am not paid for forum posts. If I write a good post, then I have been good for nothing.
Participant alexium
Participant
9,195 Views
Registered: ‎04-22-2010

Re: incomplete code review

Jump to solution

 


@eteam00 wrote:

 

I guess we're not as dumb as we look.

 


 

I guess I'm more dumb than I thought.

I was absolutely, I mean ABSOLUTELY sure that RX signal in the UART was not exactly what you call an asynchronous input to an FSM.

I understand  why inputs to FSM should be synchronized, but I don't understand why is it necessary in my case.

I did study the UART code. It doesn't try to sample the RX signal on it's edge. It makes an assumption on where the edge might be (waiting for RX line to go active (low) from previous idle (high) state). It then waits 1/8th of the RX signal period (based on the known ratio between UART clock frequency and baud rate) and samples RX at that moment. I assumed there should be no fluctuations of RX line 1/8th after it's edge. I believe the creator of this UART module assumed the same.

Even more so: like I said, I've tested the UART in the standalone project, and it always worked well.

I have no idea what was happenning there and why setting the RX signal to go through 2 FFs solved the problem. If you have an explanation, I can't  wait to hear it.

 

 


@eteam00 wrote:

 

Glad to hear your design is working better.  Even more glad if you've learned a lesson or two out of this.


I hope I did!

0 Kudos
Teacher eteam00
Teacher
9,186 Views
Registered: ‎07-21-2009

Re: incomplete code review

Jump to solution

I guess I'm more dumb than I thought.

I don't think you're dumb, but you've shown a fair bit of stubborn.

I understand  why inputs to FSM should be synchronized, but I don't understand why is it necessary in my case.

...

I have no idea what was happenning there and why setting the RX signal to go through 2 FFs solved the problem. If you have an explanation, I can't  wait to hear it.

Post your (before the fix) UART receive code, please!  This won't take long.

 

When I was young and stupid, I thought that 'the rules' didn't necessarily apply to me, because of my extraordinary design cleverness.  I spent a tremendous amount of time and effort 'proving' that I was both right and extraordinarily clever.  Once I realised that I'm not always right or extraordinarily clever (at least, not so extraordinarily clever as I hoped or imagined), I've managed to squeeze out a number of successful designs over 30+ years.

 

-- Bob Elkind

SIGNATURE:
README for newbies is here: http://forums.xilinx.com/t5/New-Users-Forum/README-first-Help-for-new-users/td-p/219369

Summary:
1. Read the manual or user guide. Have you read the manual? Can you find the manual?
2. Search the forums (and search the web) for similar topics.
3. Do not post the same question on multiple forums.
4. Do not post a new topic or question on someone else's thread, start a new thread!
5. Students: Copying code is not the same as learning to design.
6 "It does not work" is not a question which can be answered. Provide useful details (with webpage, datasheet links, please).
7. You are not charged extra fees for comments in your code.
8. I am not paid for forum posts. If I write a good post, then I have been good for nothing.
Participant alexium
Participant
9,178 Views
Registered: ‎04-22-2010

Re: incomplete code review

Jump to solution

@eteam00 wrote:
I don't think you're dumb, but you've shown a fair bit of stubborn.

To say at least...

 

 


@eteam00 wrote:
 

When I was young and stupid, I thought that 'the rules' didn't necessarily apply to me, because of my extraordinary design cleverness.  I spent a tremendous amount of time and effort trying to prove that I was both right and extraordinarily clever.  Now that I'm not always right or extraordinarily clever, I've managed to squeeze out a number of successful designs over 30+ years.

 

 


 

Point taken.

 

 


@eteam00 wrote:

Post your (before the fix) UART receive code, please!  This won't take long.

 


I hope it won't, it's in Verilog. Here you go:

 

 

`timescale 1ns / 1ps

module uart(
    input clk, // The master clock for this module
    input rst, // Synchronous reset.
    input rx, // Incoming serial line
    output tx, // Outgoing serial line
    input transmit, // Signal to transmit
    input [7:0] tx_byte, // Byte to transmit
    output received, // Indicated that a byte has been received.
    output [7:0] rx_byte, // Byte received
    output is_receiving, // Low when receive line is idle.
    output reg tx_done, // Low when transmit line is idle.
    output recv_error // Indicates error in receiving packet.
    );

parameter [7:0] CLOCK_DIVIDE = 217; // clock rate (100Mhz) / (baud rate (115200) * 4)

// States for the receiving state machine.
// These are just constants, not parameters to override.
parameter [6:0] 
    RX_IDLE = 1,
    RX_CHECK_START = 2,
    RX_READ_BITS = 4,
    RX_CHECK_STOP = 8,
    RX_DELAY_RESTART = 16,
    RX_ERROR = 32,
    RX_RECEIVED = 64;

// States for the transmitting state machine.
// Constants - do not override.
parameter [3:0] 
    TX_IDLE_CYCLE_AFTER_TRANSMISSION = 1,
    TX_IDLE = 2,
    TX_SENDING = 4,
    TX_DELAY_RESTART = 8;

reg [7:0] rx_clk_divider;
reg [7:0] tx_clk_divider;

reg [7:0] recv_state = RX_IDLE;
reg [5:0] rx_countdown;
reg [3:0] rx_bits_remaining;
reg [7:0] rx_data;

reg tx_out = 1'b1;
reg [3:0] tx_state = TX_IDLE;
reg [5:0] tx_countdown;
reg [3:0] tx_bits_remaining;
reg [7:0] tx_data; 
reg transmit_int = 0;

assign received = recv_state == RX_RECEIVED;
assign recv_error = recv_state == RX_ERROR;
assign is_receiving = recv_state != RX_IDLE;
assign rx_byte = rx_data;

assign tx = tx_out;

always @(posedge clk) begin

    if (rst) begin
        recv_state = RX_IDLE;
        tx_state = TX_IDLE;
        transmit_int = 0; 
        tx_done = 0;
        rx_clk_divider = CLOCK_DIVIDE;
        tx_clk_divider = CLOCK_DIVIDE;
    end
    
    // The clk_divider counter counts down from
    // the CLOCK_DIVIDE constant. Whenever it
    // reaches 0, 1/16 of the bit period has elapsed.
   // Countdown timers for the receiving and transmitting
    // state machines are decremented.
    rx_clk_divider = rx_clk_divider - 1;
    if (rx_clk_divider == 0) begin
        rx_clk_divider = CLOCK_DIVIDE;
        rx_countdown = rx_countdown - 1;
    end
    tx_clk_divider = tx_clk_divider - 1;
    if (tx_clk_divider == 0) begin
        tx_clk_divider = CLOCK_DIVIDE;
        tx_countdown = tx_countdown - 1;
    end
    
    // Receive state machine
    case (recv_state)
        RX_IDLE: begin
            // A low pulse on the receive line indicates the
            // start of data.
            rx_clk_divider = CLOCK_DIVIDE;
            rx_countdown = 2;
            if (rx == 0) begin
                // Wait half the period - should resume in the
                // middle of this first pulse.
                recv_state = RX_CHECK_START;
            end
        end
        RX_CHECK_START: begin
            if (rx_countdown == 0) begin
                // Check the pulse is still there
                if (rx == 0) begin
                    // Pulse still there - good
                    // Wait the bit period to resume half-way
                    // through the first bit.
                    rx_countdown = 4;
                    rx_bits_remaining = 8;
                    recv_state = RX_READ_BITS;
                end else begin
                    // Pulse lasted less than half the period -
                    // not a valid transmission.
                    recv_state = RX_ERROR;
                end
            end
        end
        RX_READ_BITS: begin
            if (rx_countdown == 0) begin
                // Should be half-way through a bit pulse here.
                // Read this bit in, wait for the next if we
                // have more to get.
                rx_data = {rx, rx_data[7:1]};
                rx_countdown = 4;
                rx_bits_remaining = rx_bits_remaining - 1;
                recv_state = rx_bits_remaining !=0 ? RX_READ_BITS : RX_CHECK_STOP;
            end
        end
        RX_CHECK_STOP: begin
            if (rx_countdown == 0) begin
                // Should resume half-way through the stop bit
                // This should be high - if not, reject the
                // transmission and signal an error.
                recv_state = rx != 0 ? RX_RECEIVED : RX_ERROR;
            end
        end
        RX_DELAY_RESTART: begin
            // Waits a set number of cycles before accepting
            // another transmission.
            recv_state = rx_countdown !=0 ? RX_DELAY_RESTART : RX_IDLE;
        end
        RX_ERROR: begin
            // There was an error receiving.
            // Raises the recv_error flag for one clock
            // cycle while in this state and then waits
            // 1 bit period before accepting another
            // transmission.
            rx_countdown = 4;
            recv_state = RX_DELAY_RESTART;
        end
        RX_RECEIVED: begin
            // Successfully received a byte.
            // Raises the received flag for one clock
            // cycle while in this state.
            recv_state = RX_IDLE;
        end
    endcase
    
    // Transmit state machine
    
    case (tx_state)  
        TX_IDLE_CYCLE_AFTER_TRANSMISSION: begin 
            tx_done = 0;  
            transmit_int = 0;
            tx_state =  TX_IDLE;
        end
        
        TX_IDLE: begin
            
            tx_countdown = 4;
            tx_bits_remaining = 0;  //! 8 -> 0
            tx_clk_divider = CLOCK_DIVIDE;
            tx_data = tx_byte;   
            if (transmit_int == 1'b1) begin
                // If the transmit flag is raised in the idle
                // state, start transmitting the current content
                // of the tx_byte input.
                // Send the initial, low pulse of 1 bit period
                // to signal the start, followed by the data
                tx_out = 0;                 //Start bit
                tx_state = TX_SENDING; 
                transmit_int = 0;
            end
            else
               transmit_int = transmit;
        end
        TX_SENDING: begin
            
            if (tx_countdown == 0) begin
                tx_countdown = 4; // Set delay to send out 1 stop bit
                if (tx_bits_remaining != 8) begin
                    tx_bits_remaining = tx_bits_remaining + 1; // !
                    //tx_out = tx_data[0];
                    tx_out = tx_data[tx_bits_remaining-1];       // !
                    //tx_data = {1'b0, tx_data[7:1]};
                    
                    //tx_state = TX_SENDING;
                end else begin
                    
                    tx_out = 1;
                    tx_state = TX_DELAY_RESTART;
                    //tx_state = TX_IDLE;
                end
            end
        end
        TX_DELAY_RESTART: begin
            // Wait until tx_countdown reaches the end before
            // we send another transmission. This covers the
            // "stop bit" delay.
            if (tx_countdown == 0) begin
                tx_state = TX_IDLE_CYCLE_AFTER_TRANSMISSION;    
                tx_done = 1;
            end
        end
    endcase
end

endmodule

 

 

0 Kudos
Teacher eteam00
Teacher
9,161 Views
Registered: ‎07-21-2009

Re: incomplete code review

Jump to solution

You have more problems in your code than you realise.  The absolute killer (async input) is #4 in the list below.

 

1.  You treat rx_countdown as a 4-state (actually, 5-state) counter (you divide the bit period into 4 phases).  But rx_countdown is a 6-bit register?

 

2.  There are conflicting (overlapping) assignment statements within the same process for rx_clk_divider and rx_countdown.  No no no no no NO.

 

3.  What's with assigning register values inside a clocked process with blocking (rather than non-blocking) assignment statements?  NO!

 

4.  state RX_CHECK_START RX_IDLE (updated/corrected) -- the odds of 8-bit register recv_state being 'updated' non-uniformly due to skews of asynchronous rx input, using a 100MHz clock, are excellent.  If skews for rx input cover a 1nS range, a 100MHz clock (10nS clock period) will result in a corrupted state machine at least 10 times (characters) out of 100.  If the rx skews are greater than 1nS, the odds of state machine corruption are even higher.

 

5.  The comment about the clk_divider counter is incorrect.  Corrected below.

// Whenever it reaches 0, 1/16 1/4 of the bit period has elapsed.

6.  The receive states are defined as 7-bit parameter values, yet the recv_state register is 8 bits.  You are assigning 7-bit values to an 8-bit register.  You are also comparing this 8-bit register to 7-bit values.

 

I didn't review the TX state machine.  This should be enough to keep you busy.

 

-- Bob Elkind

SIGNATURE:
README for newbies is here: http://forums.xilinx.com/t5/New-Users-Forum/README-first-Help-for-new-users/td-p/219369

Summary:
1. Read the manual or user guide. Have you read the manual? Can you find the manual?
2. Search the forums (and search the web) for similar topics.
3. Do not post the same question on multiple forums.
4. Do not post a new topic or question on someone else's thread, start a new thread!
5. Students: Copying code is not the same as learning to design.
6 "It does not work" is not a question which can be answered. Provide useful details (with webpage, datasheet links, please).
7. You are not charged extra fees for comments in your code.
8. I am not paid for forum posts. If I write a good post, then I have been good for nothing.
0 Kudos