Potential bug in creating state machines

Tip / Sign in to post questions, reply, level up, and achieve exciting badges. Know more

cross mob
Wilton
Level 4
Level 4
50 sign-ins 25 replies posted 5 solutions authored

I have identified a possible bug in the way UDB editor instantiates state machines with reset.  I noticed that while reset was held high, the state machine would toggle back and forth between states 0 and 1 at the clock rate.  Later I used the supplied verilog code as a basis for a different state machine I was building and noticed similar behavior.  Upon looking at the code I found the following:

always @ (posedge clock)
begin
    if((Reset) == 1'b1) begin // Reset Condition
        HeaderState <= State_0_Clear;
    end
    case(HeaderState)

That means that Reset will force it into state 0, but then fall through into the case statement, which can then change the state.  Technically is bad (possibly illegal) Verilog, because it is could be making two registered assignments in the same clock cycle.  The normal way that this would be implemented would be:

always @ (posedge clock)
begin
    if((Reset) == 1'b1) begin // Reset Condition
        HeaderState <= State_0_Clear;
    end
    else begin
    case(HeaderState)

 This makes the reset exclusive to the case statement and will hold the state machine in reset and otherwise inactive until it is released.

0 Likes
6 Replies
LeoMathews
Moderator
Moderator
Moderator
First question asked 500 replies posted 100 solutions authored

Hi @Wilton 

Can you please share your UDB project with us? So that we can look at the .cyudb file with the entire generated Verilog code. Also, can you please tell how did you verify that the state machine would toggle back and forth between states 0 and 1 at the clock rate, while reset was held high?

Thanks and Regards,
Leo

0 Likes
lock attach
Attachments are accessible only for community members.

Sure, here it is.  If you open the generated Verilog the issue is at line 176, which should have an else before the case.

As far as testing, you can see that I export S0, S1 and S2 as the bits of the state machine.  They are connected to digital output pins on the CY8CKIT-046 that I am using as a test bed while waiting for my PCBs to arrive.  This was connected to my TPS2024 digital oscilloscope, which captured the activity.  I have built two of these state machines for different parts of the project and both generated this same code structure.  The other one is the one I have spent the most time with, but I ended up needing Po, so had to import it as Verilog and adapt it.  One of the first things I did was modify the state machine, adding the else clause and the toggling between state 0 and state 1 stopped.  I haven't spent much time looking to see if it mattered whether the exit condition was true, because the code was obviously defective.  On the one I converted, state 0 was a transient initialization reset state, so the exit condition was 1'b1.  Had the exit condition not been true, it is quite possible that this bug would not manifest itself, and there are probably a lot of state machines that don't immediately exit the reset state, so the behavior may not have been noticed.  Also, there may be situations where the reset state only lasts one clock, in which case it doesn't matter.  But the bottom line is that the generated Verilog code does not match the expectation for a level sensitive reset condition on a state machine.

0 Likes
LeoMathews
Moderator
Moderator
Moderator
First question asked 500 replies posted 100 solutions authored

Hi @Wilton 

Could you please share the entire PSoC Creator Project?

Thanks and Regards,
Leo

0 Likes

I'll try and get you something next week.  The actual code that was observed is long gone.  I needed parallel out, so copied and pasted the Verilog and made the changes, including fixing this bug, and then removed the original.  It is also part of a project that is pushing the limits of the UDBs with several components in a separate library.  But I should be able to easily recreate a simple example test case with about 4 states and one datapath.

Wilton

0 Likes
lock attach
Attachments are accessible only for community members.

OK.  I took the original project, created a new component called sample , stripped out most of the unrelated stuff and built it and tested it.  Sample is a simple state machine that should be permanently locked in a reset state by the hard-coded reset variable.  The Verilog code, like my previous example, shows that the reset 'if' statement lacks an else clause.

This was run on a CY8CKIT-046.  The five test outputs are available at J18 pins 7, 9, 11, 12, 10 respectively, if I'm counting the pins correctly.  (It's the right three pins that have a header attached plus two below them that are unpopulated--on the right hand breakaway board.

When the program is downloaded and starts, you can see the state variables cycling 0, 1, 2 and back to zero again.  Based on the graphic design it should be stuck in state 0.  If reset was set low, it should advance to state 2 and be stuck there.  Indeed, if I set Reset to 0, it does go to state 2 and become stuck there.

Trying to analyze the Verilog code I find that there are two conflicting assignments to Test, one from the reset if statement and one from the state it is in (except 2).  If I recall Verilog's rules for conflicting assignments, the last one is what is actually used.  So in state 0, the Reset clause is overridden and the state goes to 1.  In state 2, the Reset clause is overridden and it goes to 2.  In state 2, there is no assignment, so the Reset clause prevails.

I believe this represents a bug in the way the state 0 graphic is implemented in Verilog.  The reset clause should be an if statement with an else clause and the case statement should be contained in the else clause.  It is a simple fix, but has significant impact on any state machine with a reset.  If the state machine spends multiple cycles in a given state and the reset happens to arrive when no exit condition is true, the existing behavior is appropriate.  However, if the reset condition occurs in the same clock cycle with an exit condition, the exit condition will prevail and the reset will be ignored.  If reset was only one clock wide, it would be totally missed.  In my case (both the contrived sample and the real application where I discovered this) the initial state always transitioned to the next state, which meant that reset always failed to keep it in the initial state.  Eventually it encounters a state that doesn't always exit and Reset can finally prevail, because it is static.

Due to the idiosyncrasy of Verilog to accept the last of multiple assignments, an alternate solution would be for the code generator to simply move the Reset if clause to the bottom of the always block.  That might be a simpler modification of the code generator, although the resulting Verilog code would be more confusing to read.

0 Likes
LeoMathews
Moderator
Moderator
Moderator
First question asked 500 replies posted 100 solutions authored

Hi @Wilton 

I will report this bug to the internal team. Thank you for noticing the bug and for your suggestion.

Thanks and Regards,
Leo

0 Likes